* Re: [PATCH] net: fix htmldocs sunrpc, clnt.c
From: David Miller @ 2009-09-24 22:46 UTC (permalink / raw)
To: randy.dunlap
Cc: jaswinder, Ricardo.Labiaga, bhalevy, andros, Trond.Myklebust,
linux-nfs, netdev, linux-kernel
In-Reply-To: <4ABB9C17.3020307@oracle.com>
From: Randy Dunlap <randy.dunlap@oracle.com>
Date: Thu, 24 Sep 2009 09:19:35 -0700
> Jaswinder Singh Rajput wrote:
>> DOCPROC Documentation/DocBook/networking.xml
>> Warning(net/sunrpc/clnt.c:647): No description found for parameter 'req'
>> Warning(net/sunrpc/clnt.c:647): No description found for parameter 'tk_ops'
>> Warning(net/sunrpc/clnt.c:647): Excess function parameter 'ops' description in 'rpc_run_bc_task'
>>
>> Signed-off-by: Jaswinder Singh Rajput <jaswinderrajput@gmail.com>
>
> Ack. Already sent, but possibly lost.
Applied.
^ permalink raw reply
* Re: [PATCH] genetlink: fix netns vs. netlink table locking (2)
From: David Miller @ 2009-09-24 22:45 UTC (permalink / raw)
To: johannes; +Cc: netdev
In-Reply-To: <1253698470.4458.48.camel@johannes.local>
From: Johannes Berg <johannes@sipsolutions.net>
Date: Wed, 23 Sep 2009 11:34:30 +0200
> Similar to commit d136f1bd366fdb7e747ca7e0218171e7a00a98a5,
> there's a bug when unregistering a generic netlink family,
> which is caught by the might_sleep() added in that commit:
>
> BUG: sleeping function called from invalid context at net/netlink/af_netlink.c:183
> in_atomic(): 1, irqs_disabled(): 0, pid: 1510, name: rmmod
> 2 locks held by rmmod/1510:
> #0: (genl_mutex){+.+.+.}, at: [<ffffffff8138283b>] genl_unregister_family+0x2b/0x130
> #1: (rcu_read_lock){.+.+..}, at: [<ffffffff8138270c>] __genl_unregister_mc_group+0x1c/0x120
> Pid: 1510, comm: rmmod Not tainted 2.6.31-wl #444
> Call Trace:
> [<ffffffff81044ff9>] __might_sleep+0x119/0x150
> [<ffffffff81380501>] netlink_table_grab+0x21/0x100
> [<ffffffff813813a3>] netlink_clear_multicast_users+0x23/0x60
> [<ffffffff81382761>] __genl_unregister_mc_group+0x71/0x120
> [<ffffffff81382866>] genl_unregister_family+0x56/0x130
> [<ffffffffa0007d85>] nl80211_exit+0x15/0x20 [cfg80211]
> [<ffffffffa000005a>] cfg80211_exit+0x1a/0x40 [cfg80211]
>
> Fix in the same way by grabbing the netlink table lock
> before doing rcu_read_lock().
>
> Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
Applied.
^ permalink raw reply
* Re: [PATCH] Phonet: fix race for port number in concurrent bind()
From: David Miller @ 2009-09-24 22:45 UTC (permalink / raw)
To: remi; +Cc: netdev, remi.denis-courmont
In-Reply-To: <1253711831-7947-1-git-send-email-remi@remlab.net>
From: Rémi Denis-Courmont <remi@remlab.net>
Date: Wed, 23 Sep 2009 16:17:10 +0300
> From: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
>
> From: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
>
> Allocating a port number to a socket and hashing that socket shall be
> an atomic operation with regards to other port allocation. Otherwise,
> we could allocate a port that is already being allocated to another
> socket.
>
> Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
Applied.
^ permalink raw reply
* Re: [PATCH] Phonet: error on broadcast sending (unimplemented)
From: David Miller @ 2009-09-24 22:45 UTC (permalink / raw)
To: remi; +Cc: netdev, remi.denis-courmont
In-Reply-To: <1253711831-7947-2-git-send-email-remi@remlab.net>
From: Rémi Denis-Courmont <remi@remlab.net>
Date: Wed, 23 Sep 2009 16:17:11 +0300
> From: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
>
> From: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
>
> If we ever implement this, then we can stop returning an error.
>
> Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
Applied.
^ permalink raw reply
* Re: [PATCH 1/2] pktgen: T_TERMINATE flag is unused
From: David Miller @ 2009-09-24 22:44 UTC (permalink / raw)
To: shemminger; +Cc: netdev
In-Reply-To: <20090923054201.440781719@vyatta.com>
From: Stephen Hemminger <shemminger@vyatta.com>
Date: Tue, 22 Sep 2009 22:41:42 -0700
> Get rid of unused flag bit.
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
Applied.
^ permalink raw reply
* Re: [PATCH 2/2] pktgen: better scheduler friendliness
From: David Miller @ 2009-09-24 22:44 UTC (permalink / raw)
To: shemminger; +Cc: netdev
In-Reply-To: <20090923054201.520300835@vyatta.com>
From: Stephen Hemminger <shemminger@vyatta.com>
Date: Tue, 22 Sep 2009 22:41:43 -0700
> Previous update did not resched in inner loop causing watchdogs.
> Rewrite inner loop to:
> * account for delays better with less clock calls
> * more accurate timing of delay:
> - only delay if packet was successfully sent
> - if delay is 100ns and it takes 10ns to build packet then
> account for that
> * use wait_event_interruptible_timeout rather than open coding it.
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
Applied.
^ permalink raw reply
* Re: [PATCH 1/2] ipv4: fix do_ip_setsockopt optlen check for IP_MULTICAST_IF
From: David Miller @ 2009-09-24 22:44 UTC (permalink / raw)
To: shanwei
Cc: dfeng, kaber, yoshfuji, jmorris, pekkas, kuznet, netdev,
linux-kernel
In-Reply-To: <4AB97CB6.4000004@cn.fujitsu.com>
From: Shan Wei <shanwei@cn.fujitsu.com>
Date: Wed, 23 Sep 2009 09:41:10 +0800
> [PATCH BUGFIX] ipv4: check optlen for IP_MULTICAST_IF option
>
> Due to man page of setsockopt, if optlen is not valid, kernel should return
> -EINVAL. But a simple testcase as following, errno is 0, which means setsockopt
> is successful.
> addr.s_addr = inet_addr("192.1.2.3");
> setsockopt(s, IPPROTO_IP, IP_MULTICAST_IF, &addr, 1);
> printf("errno is %d\n", errno);
>
> Xiaotian Feng(dfeng@redhat.com) caught the bug. We fix it firstly checking
> the availability of optlen and then dealing with the logic like other options.
>
> Reported-by: Xiaotian Feng <dfeng@redhat.com>
> Signed-off-by: Shan Wei <shanwei@cn.fujitsu.com>
> Acked-by: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
Applied.
^ permalink raw reply
* Re: [PATCH] 3c59x: Get rid of "Trying to free already-free IRQ"
From: David Miller @ 2009-09-24 22:26 UTC (permalink / raw)
To: avorontsov; +Cc: rjw, linux-pm, netdev
In-Reply-To: <20090924213039.GA15904@oksana.dev.rtsoft.ru>
From: Anton Vorontsov <avorontsov@ru.mvista.com>
Date: Fri, 25 Sep 2009 01:30:39 +0400
> On Thu, Sep 24, 2009 at 10:30:33PM +0200, Rafael J. Wysocki wrote:
>> On Thursday 24 September 2009, Anton Vorontsov wrote:
>> > Following trace pops up if we try to suspend with 3c59x ethernet NIC
>> > brought down:
>>
>> Patch looks good, but IMO it'd be a little effort to convert the driver to
>> dev_pm_ops while you're at it (please see r8169 for a working example).
>
> I'd like to avoid putting irrelevant stuff into bugfixes.
Agreed.
^ permalink raw reply
* [PATCH] icmp: No need to call sk_write_space()
From: Eric Dumazet @ 2009-09-24 22:16 UTC (permalink / raw)
To: David S. Miller; +Cc: Linux Netdev List
We can make icmp messages tx completion callback a litle bit faster.
Setting SOCK_USE_WRITE_QUEUE sk flag tells sock_wfree() to
not call sk_write_space() on a socket we know no thread is posssibly
waiting for write space. (on per cpu kernel internal icmp sockets only)
This avoids the sock_def_write_space() call and
read_lock(&sk->sk_callback_lock)/read_unlock(&sk->sk_callback_lock) calls
as well.
We avoid three atomic ops.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index 5bc13fe..84adb57 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -1165,6 +1165,10 @@ static int __net_init icmp_sk_init(struct net *net)
sk->sk_sndbuf =
(2 * ((64 * 1024) + sizeof(struct sk_buff)));
+ /*
+ * Speedup sock_wfree()
+ */
+ sock_set_flag(sk, SOCK_USE_WRITE_QUEUE);
inet_sk(sk)->pmtudisc = IP_PMTUDISC_DONT;
}
^ permalink raw reply related
* [PATCH 13/12] gigaset: add some more CAPI message handling
From: Tilman Schmidt @ 2009-09-24 22:04 UTC (permalink / raw)
To: Karsten Keil, Karsten Keil
Cc: Hansjoerg Lipp, davem, i4ldeveloper, netdev, linux-kernel
In-Reply-To: <20090924-patch-0.tilman@imap.cc>
Rejecting unsupported CAPI messages with code 0x1102 "illegal command
or subcommand" makes some real-world CAPI apps rather unhappy. This
patch adds code to reply to FACILITY_REQ and RESET_B3_REQ messages
with an appropriate "not supported" message, to accept but ignore
FACILITY_RESP, RESET_B3_RESP and MANUFACTURER messages, and to log
any messages still rejected as illegal.
Signed-off-by: Tilman Schmidt <tilman@imap.cc>
---
drivers/isdn/gigaset/capi.c | 171 +++++++++++++++++++++++++++++++++++++++---
1 files changed, 158 insertions(+), 13 deletions(-)
diff --git a/drivers/isdn/gigaset/capi.c b/drivers/isdn/gigaset/capi.c
index 6ea2b1d..8afff37 100644
--- a/drivers/isdn/gigaset/capi.c
+++ b/drivers/isdn/gigaset/capi.c
@@ -21,6 +21,7 @@
#define CapiNcpiNotSupportedByProtocol 0x0001
#define CapiFlagsNotSupportedByProtocol 0x0002
#define CapiAlertAlreadySent 0x0003
+#define CapiFacilitySpecificFunctionNotSupported 0x3011
/* missing from capicmd.h */
#define CAPI_CONNECT_IND_BASELEN (CAPI_MSG_BASELEN+4+2+8*1)
@@ -31,9 +32,19 @@
#define CAPI_DATA_B3_CONF_LEN (CAPI_MSG_BASELEN+4+2+2)
#define CAPI_DISCONNECT_IND_LEN (CAPI_MSG_BASELEN+4+2)
#define CAPI_DISCONNECT_B3_IND_BASELEN (CAPI_MSG_BASELEN+4+2+1)
+#define CAPI_FACILITY_CONF_BASELEN (CAPI_MSG_BASELEN+4+2+2+1)
/* most _CONF messages contain only Controller/PLCI/NCCI and Info parameters */
#define CAPI_STDCONF_LEN (CAPI_MSG_BASELEN+4+2)
+#define CAPI_FACILITY_HANDSET 0x0000
+#define CAPI_FACILITY_DTMF 0x0001
+#define CAPI_FACILITY_V42BIS 0x0002
+#define CAPI_FACILITY_SUPPSVC 0x0003
+#define CAPI_FACILITY_WAKEUP 0x0004
+#define CAPI_FACILITY_LI 0x0005
+
+#define CAPI_SUPPSVC_GETSUPPORTED 0x0000
+
/* missing from capiutil.h */
#define CAPIMSG_PLCI_PART(m) CAPIMSG_U8(m, 9)
#define CAPIMSG_NCCI_PART(m) CAPIMSG_U16(m, 10)
@@ -1044,6 +1055,108 @@ static void send_conf(struct gigaset_capi_ctr *iif,
}
/*
+ * process FACILITY_REQ message
+ */
+static void do_facility_req(struct gigaset_capi_ctr *iif,
+ struct gigaset_capi_appl *ap,
+ struct sk_buff *skb)
+{
+ struct cardstate *cs = iif->ctr.driverdata;
+ struct sk_buff *cskb;
+ u8 *pparam;
+ unsigned int msgsize = CAPI_FACILITY_CONF_BASELEN;
+ u16 function, info;
+ static u8 confparam[10]; /* max. 9 octets + length byte */
+
+ /* decode message */
+ capi_message2cmsg(&iif->acmsg, skb->data);
+ dump_cmsg(DEBUG_CMD, __func__, &iif->acmsg);
+
+ /*
+ * Facility Request Parameter is not decoded by capi_message2cmsg()
+ * encoding depends on Facility Selector
+ */
+ switch (iif->acmsg.FacilitySelector) {
+ case CAPI_FACILITY_DTMF: /* ToDo */
+ info = CapiFacilityNotSupported;
+ confparam[0] = 2; /* length */
+ /* DTMF information: Unknown DTMF request */
+ capimsg_setu16(confparam, 1, 2);
+ break;
+
+ case CAPI_FACILITY_V42BIS: /* not supported */
+ info = CapiFacilityNotSupported;
+ confparam[0] = 2; /* length */
+ /* V.42 bis information: not available */
+ capimsg_setu16(confparam, 1, 1);
+ break;
+
+ case CAPI_FACILITY_SUPPSVC:
+ /* decode Function parameter */
+ pparam = iif->acmsg.FacilityRequestParameter;
+ if (pparam == NULL || *pparam < 2) {
+ dev_notice(cs->dev, "%s: %s missing\n", "FACILITY_REQ",
+ "Facility Request Parameter");
+ send_conf(iif, ap, skb, CapiIllMessageParmCoding);
+ return;
+ }
+ function = CAPIMSG_U16(pparam, 1);
+ switch (function) {
+ case CAPI_SUPPSVC_GETSUPPORTED:
+ info = CapiSuccess;
+ /* Supplementary Service specific parameter */
+ confparam[3] = 6; /* length */
+ /* Supplementary services info: Success */
+ capimsg_setu16(confparam, 4, CapiSuccess);
+ /* Supported Services: none */
+ capimsg_setu32(confparam, 6, 0);
+ break;
+ /* ToDo: add supported services */
+ default:
+ info = CapiFacilitySpecificFunctionNotSupported;
+ /* Supplementary Service specific parameter */
+ confparam[3] = 2; /* length */
+ /* Supplementary services info: not supported */
+ capimsg_setu16(confparam, 4,
+ CapiSupplementaryServiceNotSupported);
+ }
+
+ /* Facility confirmation parameter */
+ confparam[0] = confparam[3] + 3; /* total length */
+ /* Function: copy from _REQ message */
+ capimsg_setu16(confparam, 1, function);
+ /* Supplementary Service specific parameter already set above */
+ break;
+
+ case CAPI_FACILITY_WAKEUP: /* ToDo */
+ info = CapiFacilityNotSupported;
+ confparam[0] = 2; /* length */
+ /* Number of accepted awake request parameters: 0 */
+ capimsg_setu16(confparam, 1, 0);
+ break;
+
+ default:
+ info = CapiFacilityNotSupported;
+ confparam[0] = 0; /* empty struct */
+ }
+
+ /* send FACILITY_CONF with given Info and confirmation parameter */
+ capi_cmsg_answer(&iif->acmsg);
+ iif->acmsg.Info = info;
+ iif->acmsg.FacilityConfirmationParameter = confparam;
+ msgsize += confparam[0]; /* length */
+ cskb = alloc_skb(msgsize, GFP_ATOMIC);
+ if (!cskb) {
+ dev_err(cs->dev, "%s: out of memory\n", __func__);
+ return;
+ }
+ capi_cmsg2message(&iif->acmsg, __skb_put(cskb, msgsize));
+ dump_cmsg(DEBUG_CMD, __func__, &iif->acmsg);
+ capi_ctr_handle_message(&iif->ctr, ap->id, cskb);
+}
+
+
+/*
* process LISTEN_REQ message
* just store the masks in the application data structure
*/
@@ -1766,7 +1879,28 @@ static void do_data_b3_req(struct gigaset_capi_ctr *iif,
}
/*
- * CAPI message handler: just reply "not supported in current state"
+ * process RESET_B3_REQ message
+ * just always reply "not supported by current protocol"
+ */
+static void do_reset_b3_req(struct gigaset_capi_ctr *iif,
+ struct gigaset_capi_appl *ap,
+ struct sk_buff *skb)
+{
+ /* decode message */
+ capi_message2cmsg(&iif->acmsg, skb->data);
+ dump_cmsg(DEBUG_CMD, __func__, &iif->acmsg);
+ send_conf(iif, ap, skb,
+ CapiResetProcedureNotSupportedByCurrentProtocol);
+}
+
+/*
+ * dump unsupported/ignored messages at most twice per minute,
+ * some apps send those very frequently
+ */
+static unsigned long ignored_msg_dump_time;
+
+/*
+ * unsupported CAPI message handler
*/
static void do_unsupported(struct gigaset_capi_ctr *iif,
struct gigaset_capi_appl *ap,
@@ -1774,7 +1908,8 @@ static void do_unsupported(struct gigaset_capi_ctr *iif,
{
/* decode message */
capi_message2cmsg(&iif->acmsg, skb->data);
- dump_cmsg(DEBUG_CMD, __func__, &iif->acmsg);
+ if (printk_timed_ratelimit(&ignored_msg_dump_time, 30 * 1000))
+ dump_cmsg(DEBUG_CMD, __func__, &iif->acmsg);
send_conf(iif, ap, skb, CapiMessageNotSupportedInCurrentState);
}
@@ -1785,7 +1920,11 @@ static void do_nothing(struct gigaset_capi_ctr *iif,
struct gigaset_capi_appl *ap,
struct sk_buff *skb)
{
- dump_rawmsg(DEBUG_CMD, __func__, skb->data);
+ if (printk_timed_ratelimit(&ignored_msg_dump_time, 30 * 1000)) {
+ /* decode message */
+ capi_message2cmsg(&iif->acmsg, skb->data);
+ dump_cmsg(DEBUG_CMD, __func__, &iif->acmsg);
+ }
dev_kfree_skb(skb);
}
@@ -1809,6 +1948,7 @@ static struct {
/* most frequent messages first for faster lookup */
{ CAPI_DATA_B3_REQ, do_data_b3_req },
{ CAPI_DATA_B3_RESP, do_data_b3_resp },
+
{ CAPI_ALERT_REQ, do_alert_req },
{ CAPI_CONNECT_ACTIVE_RESP, do_nothing },
{ CAPI_CONNECT_B3_ACTIVE_RESP, do_nothing },
@@ -1821,23 +1961,25 @@ static struct {
{ CAPI_DISCONNECT_B3_RESP, do_nothing },
{ CAPI_DISCONNECT_REQ, do_disconnect_req },
{ CAPI_DISCONNECT_RESP, do_nothing },
- { CAPI_INFO_REQ, do_unsupported },
+ { CAPI_FACILITY_REQ, do_facility_req },
+ { CAPI_FACILITY_RESP, do_nothing },
+ { CAPI_LISTEN_REQ, do_listen_req },
+ { CAPI_SELECT_B_PROTOCOL_REQ, do_unsupported },
+ { CAPI_RESET_B3_REQ, do_reset_b3_req },
+ { CAPI_RESET_B3_RESP, do_nothing },
+
/*
* ToDo: support overlap sending (requires ev-layer state
* machine extension to generate additional ATD commands)
*/
+ { CAPI_INFO_REQ, do_unsupported },
{ CAPI_INFO_RESP, do_nothing },
- { CAPI_LISTEN_REQ, do_listen_req },
- { CAPI_SELECT_B_PROTOCOL_REQ, do_unsupported },
+
/*
- * ToDo:
- * CAPI_FACILITY_REQ
- * CAPI_FACILITY_RESP
- * CAPI_MANUFACTURER_REQ
- * CAPI_MANUFACTURER_RESP
- * CAPI_RESET_B3_REQ
- * CAPI_RESET_B3_RESP
+ * ToDo: what's the proper response for these?
*/
+ { CAPI_MANUFACTURER_REQ, do_nothing },
+ { CAPI_MANUFACTURER_RESP, do_nothing },
};
/* look up handler */
@@ -1887,6 +2029,9 @@ static u16 gigaset_send_message(struct capi_ctr *ctr, struct sk_buff *skb)
handler = lookup_capi_send_handler(CAPIMSG_CMD(skb->data));
if (!handler) {
/* unknown/unsupported message type */
+ if (printk_ratelimit())
+ dev_notice(cs->dev, "%s: unsupported message %u\n",
+ __func__, CAPIMSG_CMD(skb->data));
return CAPI_ILLCMDORSUBCMDORMSGTOSMALL;
}
--
1.6.2.1.214.ge986c
^ permalink raw reply related
* [PATCH 5/4] Documentation: correction to isdn/INTERFACE.CAPI
From: Tilman Schmidt @ 2009-09-24 22:04 UTC (permalink / raw)
To: Karsten Keil, Karsten Keil; +Cc: davem, i4ldeveloper, netdev, linux-kernel
In-Reply-To: <20090924-patch-0.tilman@imap.cc>
Correct the paragraphs describing the _cstruct and _cmstruct types.
The _cstruct representation is in fact used for some struct
parameters containing struct subparameters, too.
Impact: documentation
Signed-off-by: Tilman Schmidt <tilman@imap.cc>
---
Documentation/isdn/INTERFACE.CAPI | 15 ++++++++-------
1 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/Documentation/isdn/INTERFACE.CAPI b/Documentation/isdn/INTERFACE.CAPI
index e6bb1a7..5fe8de5 100644
--- a/Documentation/isdn/INTERFACE.CAPI
+++ b/Documentation/isdn/INTERFACE.CAPI
@@ -217,18 +217,19 @@ u16 for CAPI parameters of type 'word'
u32 for CAPI parameters of type 'dword'
-_cstruct for CAPI parameters of type 'struct' not containing any
- variably-sized (struct) subparameters (eg. 'Called Party Number')
+_cstruct for CAPI parameters of type 'struct'
The member is a pointer to a buffer containing the parameter in
CAPI encoding (length + content). It may also be NULL, which will
be taken to represent an empty (zero length) parameter.
+ Subparameters are stored in encoded form within the content part.
-_cmstruct for CAPI parameters of type 'struct' containing 'struct'
- subparameters ('Additional Info' and 'B Protocol')
+_cmstruct alternative representation for CAPI parameters of type 'struct'
+ (used only for the 'Additional Info' and 'B Protocol' parameters)
The representation is a single byte containing one of the values:
- CAPI_DEFAULT: the parameter is empty
- CAPI_COMPOSE: the values of the subparameters are stored
- individually in the corresponding _cmsg structure members
+ CAPI_DEFAULT: The parameter is empty/absent.
+ CAPI_COMPOSE: The parameter is present.
+ Subparameter values are stored individually in the corresponding
+ _cmsg structure members.
Functions capi_cmsg2message() and capi_message2cmsg() are provided to convert
messages between their transport encoding described in the CAPI 2.0 standard
--
1.6.2.1.214.ge986c
^ permalink raw reply related
* Gigaset & ISDN followup patches
From: Tilman Schmidt @ 2009-09-24 22:03 UTC (permalink / raw)
To: Karsten Keil, Karsten Keil
Cc: Hansjoerg Lipp, davem, i4ldeveloper, netdev, linux-kernel
Karsten,
I'm sending you one followup patch each to the ISDN and Gigaset patch
series for kernel 2.6.32 I sent on 2009-09-19, fixing issues I found
after that submission. I've tentatively tacked them on to the end of
each series as "patch 5/4" and "13/12" but if you prefer I can respin
patch 1 of the ISDN series and patch 12 of the Gigaset series to
incorporate the changes.
Looking forward to seeing these merged,
Tilman
^ permalink raw reply
* Re: [PATCH] 3c59x: Get rid of "Trying to free already-free IRQ"
From: Anton Vorontsov @ 2009-09-24 21:30 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: David Miller, linux-pm, netdev
In-Reply-To: <200909242230.33881.rjw@sisk.pl>
On Thu, Sep 24, 2009 at 10:30:33PM +0200, Rafael J. Wysocki wrote:
> On Thursday 24 September 2009, Anton Vorontsov wrote:
> > Following trace pops up if we try to suspend with 3c59x ethernet NIC
> > brought down:
>
> Patch looks good, but IMO it'd be a little effort to convert the driver to
> dev_pm_ops while you're at it (please see r8169 for a working example).
I'd like to avoid putting irrelevant stuff into bugfixes.
Apart from delights as bisecting and revert-only-offending-piece,
keeping bugfixes small and self-sufficient helps to back-port the
fixes to stable/distro kernels. Think of not so old kernels that
don't have dev_pm_ops.
Converting this driver (and others that I'm interested in) to
dev_pm_ops is on my todo list though.
Thanks,
--
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2
^ permalink raw reply
* Re: [PATCH] net: Fix sock_wfree() race
From: Eric Dumazet @ 2009-09-24 20:49 UTC (permalink / raw)
To: Jarek Poplawski; +Cc: David Miller, albcamus, parag.lkml, linux-kernel, netdev
In-Reply-To: <4ABBD18A.70208@gmail.com>
Jarek Poplawski a écrit :
> Eric Dumazet wrote, On 09/23/2009 03:44 PM:
>
> ...
>> Here is the patch for reference :
>>
>> [PATCH] net: Fix sock_wfree() race
>>
>> Commit 2b85a34e911bf483c27cfdd124aeb1605145dc80
>> (net: No more expensive sock_hold()/sock_put() on each tx)
>> opens a window in sock_wfree() where another cpu
>> might free the socket we are working on.
>>
>> A fix is to call sk->sk_write_space(sk) while still
>> holding a reference on sk.
>>
>>
>> Reported-by: Jike Song <albcamus@gmail.com>
>> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
>> ---
>> net/core/sock.c | 19 ++++++++++++-------
>> 1 files changed, 12 insertions(+), 7 deletions(-)
>>
>> diff --git a/net/core/sock.c b/net/core/sock.c
>> index 30d5446..e1f034e 100644
>> --- a/net/core/sock.c
>> +++ b/net/core/sock.c
>> @@ -1228,17 +1228,22 @@ void __init sk_init(void)
>> void sock_wfree(struct sk_buff *skb)
>> {
>> struct sock *sk = skb->sk;
>> - int res;
>> + unsigned int len = skb->truesize;
>>
>> - /* In case it might be waiting for more memory. */
>> - res = atomic_sub_return(skb->truesize, &sk->sk_wmem_alloc);
>> - if (!sock_flag(sk, SOCK_USE_WRITE_QUEUE))
>> + if (!sock_flag(sk, SOCK_USE_WRITE_QUEUE)) {
>> + /*
>> + * Keep a reference on sk_wmem_alloc, this will be released
>> + * after sk_write_space() call
>> + */
>> + atomic_sub(len - 1, &sk->sk_wmem_alloc);
>> sk->sk_write_space(sk);
>> + len = 1;
>> + }
>> /*
>> - * if sk_wmem_alloc reached 0, we are last user and should
>> - * free this sock, as sk_free() call could not do it.
>> + * if sk_wmem_alloc reaches 0, we must finish what sk_free()
>> + * could not do because of in-flight packets
>> */
>> - if (res == 0)
>> + if (atomic_sub_return(len, &sk->sk_wmem_alloc) == 0)
>> __sk_free(sk);
>
>
> Probably atomic_sub_and_test() is more popular for this.
Indeed, as x86 can generate faster code (no need of xadd instruction)
Thanks Jarek
[PATCH] net: Fix sock_wfree() race
Commit 2b85a34e911bf483c27cfdd124aeb1605145dc80
(net: No more expensive sock_hold()/sock_put() on each tx)
opens a window in sock_wfree() where another cpu
might free the socket we are working on.
A fix is to call sk->sk_write_space(sk) while still
holding a reference on sk.
Reported-by: Jike Song <albcamus@gmail.com>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
net/core/sock.c | 19 ++++++++++++-------
1 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/net/core/sock.c b/net/core/sock.c
index 30d5446..e1f034e 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1228,17 +1228,22 @@ void __init sk_init(void)
void sock_wfree(struct sk_buff *skb)
{
struct sock *sk = skb->sk;
- int res;
+ unsigned int len = skb->truesize;
- /* In case it might be waiting for more memory. */
- res = atomic_sub_return(skb->truesize, &sk->sk_wmem_alloc);
- if (!sock_flag(sk, SOCK_USE_WRITE_QUEUE))
+ if (!sock_flag(sk, SOCK_USE_WRITE_QUEUE)) {
+ /*
+ * Keep a reference on sk_wmem_alloc, this will be released
+ * after sk_write_space() call
+ */
+ atomic_sub(len - 1, &sk->sk_wmem_alloc);
sk->sk_write_space(sk);
+ len = 1;
+ }
/*
- * if sk_wmem_alloc reached 0, we are last user and should
- * free this sock, as sk_free() call could not do it.
+ * if sk_wmem_alloc reaches 0, we must finish what sk_free()
+ * could not do because of in-flight packets
*/
- if (res == 0)
+ if (atomic_sub_and_test(len, &sk->sk_wmem_alloc))
__sk_free(sk);
}
EXPORT_SYMBOL(sock_wfree);
^ permalink raw reply related
* Re: [PATCH] inet_peer: Optimize inet_getid()
From: Eric Dumazet @ 2009-09-24 20:44 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David S. Miller, Linux Netdev List
In-Reply-To: <20090924123020.59a2ee6e@s6510>
Stephen Hemminger a écrit :
> On Thu, 24 Sep 2009 21:04:56 +0200
> Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>> While investigating for network latencies, I found inet_getid() was a contention point
>> for some workloads.
>>
>> Fix is straightforward, using cmpxchg() instead of
>> a spin_lock_bh()/spin_unlock_bh() pair on a central lock.
>>
>> Another possibility was to use an atomic_t and atomic_add_return() but
>> the size of struct inet_peer object would had doubled on x86_64 because of
>> SLAB_HWCACHE_ALIGN constraint.
>>
>> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
>
> I thought cmpxchg was not available on all architectures.
Here is the second version of the patch, thanks Stephen
[PATCH] inet_peer: Optimize inet_getid()
While investigating for network latencies, I found inet_getid() was a contention point
for some workloads.
If __HAVE_ARCH_CMPXCHG is defined, we can use cmpxchg() instead of
a spin_lock_bh()/spin_unlock_bh() pair on a central lock.
On other arches, we can use an atomic_t instead of a u16,
and atomic_add_return(). This might grow memory usage a bit, unless
someone invents atomic16_t :)
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
include/net/inetpeer.h | 43 ++++++++++++++++++++++++++++++++-------
net/ipv4/inetpeer.c | 5 ----
net/ipv4/route.c | 2 -
3 files changed, 38 insertions(+), 12 deletions(-)
diff --git a/include/net/inetpeer.h b/include/net/inetpeer.h
index 15e1f8f..f1cde51 100644
--- a/include/net/inetpeer.h
+++ b/include/net/inetpeer.h
@@ -19,7 +19,11 @@ struct inet_peer
struct inet_peer *avl_left, *avl_right;
__be32 v4daddr; /* peer's address */
__u16 avl_height;
+#if defined(__HAVE_ARCH_CMPXCHG)
__u16 ip_id_count; /* IP ID for the next packet */
+#else
+ atomic_t ip_id_count;
+#endif
struct list_head unused;
__u32 dtime; /* the time of last use of not
* referenced entries */
@@ -37,17 +41,42 @@ struct inet_peer *inet_getpeer(__be32 daddr, int create);
/* can be called from BH context or outside */
extern void inet_putpeer(struct inet_peer *p);
-extern spinlock_t inet_peer_idlock;
/* can be called with or without local BH being disabled */
static inline __u16 inet_getid(struct inet_peer *p, int more)
{
- __u16 id;
+ __u16 old;
- spin_lock_bh(&inet_peer_idlock);
- id = p->ip_id_count;
- p->ip_id_count += 1 + more;
- spin_unlock_bh(&inet_peer_idlock);
- return id;
+ more++;
+#if defined(__HAVE_ARCH_CMPXCHG)
+ while (1) {
+ old = p->ip_id_count;
+ if (cmpxchg(&p->ip_id_count, old, old + more) == old)
+ break;
+ }
+#else
+ old = atomic_add_return(more, &p->ip_id_count) - more;
+#endif
+ return old;
}
+static inline void inet_id_set(struct inet_peer *p, int val)
+{
+#if defined(__HAVE_ARCH_CMPXCHG)
+ p->ip_id_count = val;
+#else
+ atomic_set(&p->ip_id_count, val);
+#endif
+}
+
+static inline __u16 inet_id_read(const struct inet_peer *p)
+{
+#if defined(__HAVE_ARCH_CMPXCHG)
+ return p->ip_id_count;
+#else
+ return atomic_read(&p->ip_id_count);
+#endif
+}
+
+
+
#endif /* _NET_INETPEER_H */
diff --git a/net/ipv4/inetpeer.c b/net/ipv4/inetpeer.c
index b1fbe18..3641831 100644
--- a/net/ipv4/inetpeer.c
+++ b/net/ipv4/inetpeer.c
@@ -67,9 +67,6 @@
* ip_id_count: idlock
*/
-/* Exported for inet_getid inline function. */
-DEFINE_SPINLOCK(inet_peer_idlock);
-
static struct kmem_cache *peer_cachep __read_mostly;
#define node_height(x) x->avl_height
@@ -390,7 +387,7 @@ struct inet_peer *inet_getpeer(__be32 daddr, int create)
n->v4daddr = daddr;
atomic_set(&n->refcnt, 1);
atomic_set(&n->rid, 0);
- n->ip_id_count = secure_ip_id(daddr);
+ inet_id_set(n, secure_ip_id(daddr));
n->tcp_ts_stamp = 0;
write_lock_bh(&peer_pool_lock);
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index df93473..a090fcf 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -2855,7 +2855,7 @@ static int rt_fill_info(struct net *net,
error = rt->u.dst.error;
expires = rt->u.dst.expires ? rt->u.dst.expires - jiffies : 0;
if (rt->peer) {
- id = rt->peer->ip_id_count;
+ id = inet_id_read(rt->peer);
if (rt->peer->tcp_ts_stamp) {
ts = rt->peer->tcp_ts;
tsage = get_seconds() - rt->peer->tcp_ts_stamp;
^ permalink raw reply related
* Re: [PATCH] 3c59x: Get rid of "Trying to free already-free IRQ"
From: Rafael J. Wysocki @ 2009-09-24 20:30 UTC (permalink / raw)
To: Anton Vorontsov; +Cc: netdev, linux-pm, David Miller
In-Reply-To: <20090924183152.GA30254@oksana.dev.rtsoft.ru>
On Thursday 24 September 2009, Anton Vorontsov wrote:
> Following trace pops up if we try to suspend with 3c59x ethernet NIC
> brought down:
Patch looks good, but IMO it'd be a little effort to convert the driver to
dev_pm_ops while you're at it (please see r8169 for a working example).
Thanks,
Rafael
> root@b1:~# ifconfig eth16 down
> root@b1:~# echo mem > /sys/power/state
> ...
> 3c59x 0000:00:10.0: suspend
> 3c59x 0000:00:10.0: PME# disabled
> Trying to free already-free IRQ 48
> ------------[ cut here ]------------
> Badness at c00554e4 [verbose debug info unavailable]
> NIP: c00554e4 LR: c00554e4 CTR: c019a098
> REGS: c7975c60 TRAP: 0700 Not tainted (2.6.31-rc4)
> MSR: 00021032 <ME,CE,IR,DR> CR: 28242422 XER: 20000000
> TASK = c79cb0c0[1746] 'bash' THREAD: c7974000
> ...
> NIP [c00554e4] __free_irq+0x108/0x1b0
> LR [c00554e4] __free_irq+0x108/0x1b0
> Call Trace:
> [c7975d10] [c00554e4] __free_irq+0x108/0x1b0 (unreliable)
> [c7975d30] [c005559c] free_irq+0x10/0x24
> [c7975d40] [c01e21ec] vortex_suspend+0x70/0xc4
> [c7975d60] [c017e584] pci_legacy_suspend+0x58/0x100
>
> This is because the driver manages interrupts without checking for
> netif_running().
>
> Though, there are few other issues with suspend/resume in this driver.
> The intention of calling free_irq() in suspend() was to avoid any
> possible spurious interrupts (see commit 5b039e681b8c5f30aac9cc04385
> "3c59x PM fixes"). But,
>
> - On resume, the driver was requesting IRQ just after pci_set_master(),
> but before vortex_up() (which actually resets 3c59x chips).
>
> - Issuing free_irq() on a shared IRQ doesn't guarantee that a buggy
> HW won't trigger spurious interrupts in another driver that
> requested the same interrupt. So, if we want to protect from
> unexpected interrupts, then on suspend we should issue disable_irq(),
> not free_irq().
>
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> ---
> drivers/net/3c59x.c | 12 +++---------
> 1 files changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/3c59x.c b/drivers/net/3c59x.c
> index c34aee9..7cdd4b0 100644
> --- a/drivers/net/3c59x.c
> +++ b/drivers/net/3c59x.c
> @@ -807,10 +807,10 @@ static int vortex_suspend(struct pci_dev *pdev, pm_message_t state)
> if (netif_running(dev)) {
> netif_device_detach(dev);
> vortex_down(dev, 1);
> + disable_irq(dev->irq);
> }
> pci_save_state(pdev);
> pci_enable_wake(pdev, pci_choose_state(pdev, state), 0);
> - free_irq(dev->irq, dev);
> pci_disable_device(pdev);
> pci_set_power_state(pdev, pci_choose_state(pdev, state));
> }
> @@ -833,18 +833,12 @@ static int vortex_resume(struct pci_dev *pdev)
> return err;
> }
> pci_set_master(pdev);
> - if (request_irq(dev->irq, vp->full_bus_master_rx ?
> - &boomerang_interrupt : &vortex_interrupt, IRQF_SHARED, dev->name, dev)) {
> - pr_warning("%s: Could not reserve IRQ %d\n", dev->name, dev->irq);
> - pci_disable_device(pdev);
> - return -EBUSY;
> - }
> if (netif_running(dev)) {
> err = vortex_up(dev);
> if (err)
> return err;
> - else
> - netif_device_attach(dev);
> + enable_irq(dev->irq);
> + netif_device_attach(dev);
> }
> }
> return 0;
^ permalink raw reply
* Re: [PATCH] net: Fix sock_wfree() race
From: Jarek Poplawski @ 2009-09-24 20:07 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, albcamus, parag.lkml, linux-kernel, netdev
In-Reply-To: <4ABA262F.9040407@gmail.com>
Eric Dumazet wrote, On 09/23/2009 03:44 PM:
...
> Here is the patch for reference :
>
> [PATCH] net: Fix sock_wfree() race
>
> Commit 2b85a34e911bf483c27cfdd124aeb1605145dc80
> (net: No more expensive sock_hold()/sock_put() on each tx)
> opens a window in sock_wfree() where another cpu
> might free the socket we are working on.
>
> A fix is to call sk->sk_write_space(sk) while still
> holding a reference on sk.
>
>
> Reported-by: Jike Song <albcamus@gmail.com>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> ---
> net/core/sock.c | 19 ++++++++++++-------
> 1 files changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/net/core/sock.c b/net/core/sock.c
> index 30d5446..e1f034e 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -1228,17 +1228,22 @@ void __init sk_init(void)
> void sock_wfree(struct sk_buff *skb)
> {
> struct sock *sk = skb->sk;
> - int res;
> + unsigned int len = skb->truesize;
>
> - /* In case it might be waiting for more memory. */
> - res = atomic_sub_return(skb->truesize, &sk->sk_wmem_alloc);
> - if (!sock_flag(sk, SOCK_USE_WRITE_QUEUE))
> + if (!sock_flag(sk, SOCK_USE_WRITE_QUEUE)) {
> + /*
> + * Keep a reference on sk_wmem_alloc, this will be released
> + * after sk_write_space() call
> + */
> + atomic_sub(len - 1, &sk->sk_wmem_alloc);
> sk->sk_write_space(sk);
> + len = 1;
> + }
> /*
> - * if sk_wmem_alloc reached 0, we are last user and should
> - * free this sock, as sk_free() call could not do it.
> + * if sk_wmem_alloc reaches 0, we must finish what sk_free()
> + * could not do because of in-flight packets
> */
> - if (res == 0)
> + if (atomic_sub_return(len, &sk->sk_wmem_alloc) == 0)
> __sk_free(sk);
Probably atomic_sub_and_test() is more popular for this.
Jarek P.
> }
> EXPORT_SYMBOL(sock_wfree);
>
> --
> 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] inet_peer: Optimize inet_getid()
From: Eric Dumazet @ 2009-09-24 19:57 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David S. Miller, Linux Netdev List
In-Reply-To: <20090924123020.59a2ee6e@s6510>
Stephen Hemminger a écrit :
> On Thu, 24 Sep 2009 21:04:56 +0200
> Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>> While investigating for network latencies, I found inet_getid() was a contention point
>> for some workloads.
>>
>> Fix is straightforward, using cmpxchg() instead of
>> a spin_lock_bh()/spin_unlock_bh() pair on a central lock.
>>
>> Another possibility was to use an atomic_t and atomic_add_return() but
>> the size of struct inet_peer object would had doubled on x86_64 because of
>> SLAB_HWCACHE_ALIGN constraint.
>>
>> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
>
> I thought cmpxchg was not available on all architectures.
Good point Stephen, I forgot about non x86 arches ;)
I'll send an update with cmpxchg() if available, and atomic_t as fallback.
^ permalink raw reply
* Re: question on raw sockets and source IP address validation
From: Chris Friesen @ 2009-09-24 19:37 UTC (permalink / raw)
To: Neil Horman; +Cc: Linux Network Development list
In-Reply-To: <20090924192651.GC19787@hmsreliant.think-freely.org>
On 09/24/2009 01:26 PM, Neil Horman wrote:
> That said, its not doing source validation, your socket is actually doing a
> route lookup on the flow from your specified source address to your destination
> address. So you should be able to fool the socket into doing the lookup by
> adding a route to your routing table from your source address to your
> destination address via the interface that you want to send the frames out of.
Hmm...that's an interesting point. Worth investigating for sure.
Thanks,
Chris
^ permalink raw reply
* Re: [PATCH] inet_peer: Optimize inet_getid()
From: Stephen Hemminger @ 2009-09-24 19:30 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S. Miller, Linux Netdev List
In-Reply-To: <4ABBC2D8.2040901@gmail.com>
On Thu, 24 Sep 2009 21:04:56 +0200
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> While investigating for network latencies, I found inet_getid() was a contention point
> for some workloads.
>
> Fix is straightforward, using cmpxchg() instead of
> a spin_lock_bh()/spin_unlock_bh() pair on a central lock.
>
> Another possibility was to use an atomic_t and atomic_add_return() but
> the size of struct inet_peer object would had doubled on x86_64 because of
> SLAB_HWCACHE_ALIGN constraint.
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
I thought cmpxchg was not available on all architectures.
^ permalink raw reply
* Re: [PATCHv5 3/3] vhost_net: a kernel-level virtio server
From: Ira W. Snyder @ 2009-09-24 19:27 UTC (permalink / raw)
To: Avi Kivity
Cc: Gregory Haskins, Michael S. Tsirkin, netdev, virtualization, kvm,
linux-kernel, mingo, linux-mm, akpm, hpa, Rusty Russell, s.hetze,
alacrityvm-devel
In-Reply-To: <4ABB1D44.5000007@redhat.com>
On Thu, Sep 24, 2009 at 10:18:28AM +0300, Avi Kivity wrote:
> On 09/24/2009 12:15 AM, Gregory Haskins wrote:
> >
> >>> There are various aspects about designing high-performance virtual
> >>> devices such as providing the shortest paths possible between the
> >>> physical resources and the consumers. Conversely, we also need to
> >>> ensure that we meet proper isolation/protection guarantees at the same
> >>> time. What this means is there are various aspects to any
> >>> high-performance PV design that require to be placed in-kernel to
> >>> maximize the performance yet properly isolate the guest.
> >>>
> >>> For instance, you are required to have your signal-path (interrupts and
> >>> hypercalls), your memory-path (gpa translation), and
> >>> addressing/isolation model in-kernel to maximize performance.
> >>>
> >>>
> >> Exactly. That's what vhost puts into the kernel and nothing more.
> >>
> > Actually, no. Generally, _KVM_ puts those things into the kernel, and
> > vhost consumes them. Without KVM (or something equivalent), vhost is
> > incomplete. One of my goals with vbus is to generalize the "something
> > equivalent" part here.
> >
>
> I don't really see how vhost and vbus are different here. vhost expects
> signalling to happen through a couple of eventfds and requires someone
> to supply them and implement kernel support (if needed). vbus requires
> someone to write a connector to provide the signalling implementation.
> Neither will work out-of-the-box when implementing virtio-net over
> falling dominos, for example.
>
> >>> Vbus accomplishes its in-kernel isolation model by providing a
> >>> "container" concept, where objects are placed into this container by
> >>> userspace. The host kernel enforces isolation/protection by using a
> >>> namespace to identify objects that is only relevant within a specific
> >>> container's context (namely, a "u32 dev-id"). The guest addresses the
> >>> objects by its dev-id, and the kernel ensures that the guest can't
> >>> access objects outside of its dev-id namespace.
> >>>
> >>>
> >> vhost manages to accomplish this without any kernel support.
> >>
> > No, vhost manages to accomplish this because of KVMs kernel support
> > (ioeventfd, etc). Without a KVM-like in-kernel support, vhost is a
> > merely a kind of "tuntap"-like clone signalled by eventfds.
> >
>
> Without a vbus-connector-falling-dominos, vbus-venet can't do anything
> either. Both vhost and vbus need an interface, vhost's is just narrower
> since it doesn't do configuration or enumeration.
>
> > This goes directly to my rebuttal of your claim that vbus places too
> > much in the kernel. I state that, one way or the other, address decode
> > and isolation _must_ be in the kernel for performance. Vbus does this
> > with a devid/container scheme. vhost+virtio-pci+kvm does it with
> > pci+pio+ioeventfd.
> >
>
> vbus doesn't do kvm guest address decoding for the fast path. It's
> still done by ioeventfd.
>
> >> The guest
> >> simply has not access to any vhost resources other than the guest->host
> >> doorbell, which is handed to the guest outside vhost (so it's somebody
> >> else's problem, in userspace).
> >>
> > You mean _controlled_ by userspace, right? Obviously, the other side of
> > the kernel still needs to be programmed (ioeventfd, etc). Otherwise,
> > vhost would be pointless: e.g. just use vanilla tuntap if you don't need
> > fast in-kernel decoding.
> >
>
> Yes (though for something like level-triggered interrupts we're probably
> keeping it in userspace, enjoying the benefits of vhost data path while
> paying more for signalling).
>
> >>> All that is required is a way to transport a message with a "devid"
> >>> attribute as an address (such as DEVCALL(devid)) and the framework
> >>> provides the rest of the decode+execute function.
> >>>
> >>>
> >> vhost avoids that.
> >>
> > No, it doesn't avoid it. It just doesn't specify how its done, and
> > relies on something else to do it on its behalf.
> >
>
> That someone else can be in userspace, apart from the actual fast path.
>
> > Conversely, vbus specifies how its done, but not how to transport the
> > verb "across the wire". That is the role of the vbus-connector abstraction.
> >
>
> So again, vbus does everything in the kernel (since it's so easy and
> cheap) but expects a vbus-connector. vhost does configuration in
> userspace (since it's so clunky and fragile) but expects a couple of
> eventfds.
>
> >>> Contrast this to vhost+virtio-pci (called simply "vhost" from here).
> >>>
> >>>
> >> It's the wrong name. vhost implements only the data path.
> >>
> > Understood, but vhost+virtio-pci is what I am contrasting, and I use
> > "vhost" for short from that point on because I am too lazy to type the
> > whole name over and over ;)
> >
>
> If you #define A A+B+C don't expect intelligent conversation afterwards.
>
> >>> It is not immune to requiring in-kernel addressing support either, but
> >>> rather it just does it differently (and its not as you might expect via
> >>> qemu).
> >>>
> >>> Vhost relies on QEMU to render PCI objects to the guest, which the guest
> >>> assigns resources (such as BARs, interrupts, etc).
> >>>
> >> vhost does not rely on qemu. It relies on its user to handle
> >> configuration. In one important case it's qemu+pci. It could just as
> >> well be the lguest launcher.
> >>
> > I meant vhost=vhost+virtio-pci here. Sorry for the confusion.
> >
> > The point I am making specifically is that vhost in general relies on
> > other in-kernel components to function. I.e. It cannot function without
> > having something like the PCI model to build an IO namespace. That
> > namespace (in this case, pio addresses+data tuples) are used for the
> > in-kernel addressing function under KVM + virtio-pci.
> >
> > The case of the lguest launcher is a good one to highlight. Yes, you
> > can presumably also use lguest with vhost, if the requisite facilities
> > are exposed to lguest-bus, and some eventfd based thing like ioeventfd
> > is written for the host (if it doesnt exist already).
> >
> > And when the next virt design "foo" comes out, it can make a "foo-bus"
> > model, and implement foo-eventfd on the backend, etc, etc.
> >
>
> It's exactly the same with vbus needing additional connectors for
> additional transports.
>
> > Ira can make ira-bus, and ira-eventfd, etc, etc.
> >
> > Each iteration will invariably introduce duplicated parts of the stack.
> >
>
> Invariably? Use libraries (virtio-shmem.ko, libvhost.so).
>
Referencing libraries that don't yet exist doesn't seem like a good
argument against vbus from my point of view. I'm not speficially
advocating for vbus; I'm just letting you know how it looks to another
developer in the trenches.
If you'd like to see the amount of duplication present, look at the code
I'm currently working on. It mostly works at this point, though I
haven't finished my userspace, nor figured out how to actually transfer
data.
The current question I have (just to let you know where I am in
development) is:
I have the physical address of the remote data, but how do I get it into
a userspace buffer, so I can pass it to tun?
http://www.mmarray.org/~iws/virtio-phys/
Ira
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: question on raw sockets and source IP address validation
From: Neil Horman @ 2009-09-24 19:26 UTC (permalink / raw)
To: Chris Friesen; +Cc: Linux Network Development list
In-Reply-To: <4ABBB223.8090700@nortel.com>
On Thu, Sep 24, 2009 at 11:53:39AM -0600, Chris Friesen wrote:
> Hi all,
>
> Normally when sending a packet on a SOCK_RAW socket the source IP
> address is validated against the addresses configured on the host. If
> the address isn't configured, the packet isn't sent.
>
> This can be avoided by setting IP_HDRINCL, but then the app needs to
> handle all the fragmentation itself.
>
> Is there any way to bypass the source address validation without IP_HDRINCL?
>
Nope, not with socket(AF_INET, SOCK_RAW, ...). its an IPv4 socket, so you get
ipv4 routing. If you don't want the ipv4 behavior, you can always use
AF_PACKET, to send raw frames direct to network interfaces. Of course, thats
going to imply that you do all your ip level fragmentation yourself as well.
That said, its not doing source validation, your socket is actually doing a
route lookup on the flow from your specified source address to your destination
address. So you should be able to fool the socket into doing the lookup by
adding a route to your routing table from your source address to your
destination address via the interface that you want to send the frames out of.
Regards
Neil
> Thanks,
>
> Chris
> --
> 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: ixgbe patch to provide NIC's tx/rx counters via ethtool
From: Jeff Kirsher @ 2009-09-24 19:10 UTC (permalink / raw)
To: Peter P Waskiewicz Jr; +Cc: Ben Greear, NetDev
In-Reply-To: <1253816908.3153.4.camel@localhost.localdomain>
On Thu, Sep 24, 2009 at 11:28, Peter P Waskiewicz Jr
<peter.p.waskiewicz.jr@intel.com> wrote:
> On Wed, 2009-09-23 at 15:36 -0700, Ben Greear wrote:
>> When LRO is enabled, the received packet and byte counters represent the
>> LRO'd packets, not the packets/bytes on the wire. The Intel 82599 NIC has
>> registers that keep count of the physical packets. Add these counters to
>> the ethtool stats. The byte counters are 36-bit, but the high 4 bits were
>> being ignored in the 2.6.31 ixgbe driver: Read those as well to allow
>> longer time between polling the stats to detect wraps.
>>
>> Signed-off-by: Ben Greear <greearb@candelatech.com>
>>
>>
>> Please do not apply this until the ixgbe authors ACK it. There may
>> have been reasons for not reading the high 4 bits, or they may dislike
>> this approach entirely.
>
> Aside from the trivial line-wrap on the comments, I'm fine with this
> patch. There is no issue I could find with the hardware that would
> limit you from reading the high 4 bits. And since we're reading it
> already to clear the register, we might as well use the value we get
> from it.
>
> Acked-by: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
>
I have added this patch to my tree and will push along with my other
ixgbe patches to dave/netdev. Thanks.
--
Cheers,
Jeff
^ permalink raw reply
* Re: [PATCH net-next-2.6] ixgbe: correct the parameter description
From: Jeff Kirsher @ 2009-09-24 19:08 UTC (permalink / raw)
To: Peter P Waskiewicz Jr
Cc: Jiri Pirko, davem@davemloft.net, netdev@vger.kernel.org
In-Reply-To: <1253816772.3153.2.camel@localhost.localdomain>
On Thu, Sep 24, 2009 at 11:26, Peter P Waskiewicz Jr
<peter.p.waskiewicz.jr@intel.com> wrote:
> On Thu, 2009-09-24 at 07:36 -0700, Jiri Pirko wrote:
>> ccffad25b5136958d4769ed6de5e87992dd9c65c changed parameters for function
>> ixgbe_update_uc_addr_list_generic but parameter description was not updated.
>> This patch corrects it.
>>
>> Signed-off-by: Jiri Pirko <jpirko@redhat.com>
>
> Thanks Jiri,
>
> Acked-by: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
>
Jiri/Dave-
I already have a few ixgbe patches in my queue, so I will add this
patch to my queue and push it along with my other patches to
Dave/netdev.
--
Cheers,
Jeff
^ permalink raw reply
* [PATCH] inet_peer: Optimize inet_getid()
From: Eric Dumazet @ 2009-09-24 19:04 UTC (permalink / raw)
To: David S. Miller; +Cc: Linux Netdev List
While investigating for network latencies, I found inet_getid() was a contention point
for some workloads.
Fix is straightforward, using cmpxchg() instead of
a spin_lock_bh()/spin_unlock_bh() pair on a central lock.
Another possibility was to use an atomic_t and atomic_add_return() but
the size of struct inet_peer object would had doubled on x86_64 because of
SLAB_HWCACHE_ALIGN constraint.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
include/net/inetpeer.h | 16 ++++++++--------
net/ipv4/inetpeer.c | 3 ---
2 files changed, 8 insertions(+), 11 deletions(-)
diff --git a/include/net/inetpeer.h b/include/net/inetpeer.h
index 15e1f8f..952f0ad 100644
--- a/include/net/inetpeer.h
+++ b/include/net/inetpeer.h
@@ -37,17 +37,17 @@ struct inet_peer *inet_getpeer(__be32 daddr, int create);
/* can be called from BH context or outside */
extern void inet_putpeer(struct inet_peer *p);
-extern spinlock_t inet_peer_idlock;
/* can be called with or without local BH being disabled */
static inline __u16 inet_getid(struct inet_peer *p, int more)
{
- __u16 id;
-
- spin_lock_bh(&inet_peer_idlock);
- id = p->ip_id_count;
- p->ip_id_count += 1 + more;
- spin_unlock_bh(&inet_peer_idlock);
- return id;
+ __u16 old;
+
+ while (1) {
+ old = p->ip_id_count;
+ if (cmpxchg(&p->ip_id_count, old, old + 1 + more) == old)
+ break;
+ }
+ return old;
}
#endif /* _NET_INETPEER_H */
diff --git a/net/ipv4/inetpeer.c b/net/ipv4/inetpeer.c
index b1fbe18..5dc29b8 100644
--- a/net/ipv4/inetpeer.c
+++ b/net/ipv4/inetpeer.c
@@ -67,9 +67,6 @@
* ip_id_count: idlock
*/
-/* Exported for inet_getid inline function. */
-DEFINE_SPINLOCK(inet_peer_idlock);
-
static struct kmem_cache *peer_cachep __read_mostly;
#define node_height(x) x->avl_height
^ permalink raw reply related
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