* [patch v3 09/11] xfrm: pass xfrm_user module to netlink_dump_start
From: Gao feng @ 2012-10-04 4:41 UTC (permalink / raw)
To: davem, eric.dumazet, steffen.klassert
Cc: netfilter-devel, linux-rdma, netdev, linux-crypto, pablo,
stephen.hemminger, jengelh, Gao feng
In-Reply-To: <1349325689-16104-1-git-send-email-gaofeng@cn.fujitsu.com>
set netlink_dump_control.module to avoid panic.
Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
---
net/xfrm/xfrm_user.c | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 421f984..3d909f7 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -2308,17 +2308,20 @@ static struct xfrm_link {
int (*doit)(struct sk_buff *, struct nlmsghdr *, struct nlattr **);
int (*dump)(struct sk_buff *, struct netlink_callback *);
int (*done)(struct netlink_callback *);
+ struct module *module;
} xfrm_dispatch[XFRM_NR_MSGTYPES] = {
[XFRM_MSG_NEWSA - XFRM_MSG_BASE] = { .doit = xfrm_add_sa },
[XFRM_MSG_DELSA - XFRM_MSG_BASE] = { .doit = xfrm_del_sa },
[XFRM_MSG_GETSA - XFRM_MSG_BASE] = { .doit = xfrm_get_sa,
.dump = xfrm_dump_sa,
- .done = xfrm_dump_sa_done },
+ .done = xfrm_dump_sa_done,
+ .module = THIS_MODULE },
[XFRM_MSG_NEWPOLICY - XFRM_MSG_BASE] = { .doit = xfrm_add_policy },
[XFRM_MSG_DELPOLICY - XFRM_MSG_BASE] = { .doit = xfrm_get_policy },
[XFRM_MSG_GETPOLICY - XFRM_MSG_BASE] = { .doit = xfrm_get_policy,
.dump = xfrm_dump_policy,
- .done = xfrm_dump_policy_done },
+ .done = xfrm_dump_policy_done,
+ .module = THIS_MODULE },
[XFRM_MSG_ALLOCSPI - XFRM_MSG_BASE] = { .doit = xfrm_alloc_userspi },
[XFRM_MSG_ACQUIRE - XFRM_MSG_BASE] = { .doit = xfrm_add_acquire },
[XFRM_MSG_EXPIRE - XFRM_MSG_BASE] = { .doit = xfrm_add_sa_expire },
@@ -2362,6 +2365,7 @@ static int xfrm_user_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
struct netlink_dump_control c = {
.dump = link->dump,
.done = link->done,
+ .module = link->module,
};
return netlink_dump_start(net->xfrm.nlsk, skb, nlh, &c);
}
--
1.7.7.6
^ permalink raw reply related
* [patch v3 08/11] crypto: pass crypto_user module to netlink_dump_start
From: Gao feng @ 2012-10-04 4:41 UTC (permalink / raw)
To: davem, eric.dumazet, steffen.klassert
Cc: netfilter-devel, linux-rdma, netdev, linux-crypto, pablo,
stephen.hemminger, jengelh, Gao feng, Herbert Xu
In-Reply-To: <1349325689-16104-1-git-send-email-gaofeng@cn.fujitsu.com>
set netlink_dump_control.done to avoid panic.
Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
---
crypto/crypto_user.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/crypto/crypto_user.c b/crypto/crypto_user.c
index 6bba414..f08297b 100644
--- a/crypto/crypto_user.c
+++ b/crypto/crypto_user.c
@@ -430,13 +430,15 @@ static struct crypto_link {
int (*doit)(struct sk_buff *, struct nlmsghdr *, struct nlattr **);
int (*dump)(struct sk_buff *, struct netlink_callback *);
int (*done)(struct netlink_callback *);
+ struct module *module;
} crypto_dispatch[CRYPTO_NR_MSGTYPES] = {
[CRYPTO_MSG_NEWALG - CRYPTO_MSG_BASE] = { .doit = crypto_add_alg},
[CRYPTO_MSG_DELALG - CRYPTO_MSG_BASE] = { .doit = crypto_del_alg},
[CRYPTO_MSG_UPDATEALG - CRYPTO_MSG_BASE] = { .doit = crypto_update_alg},
[CRYPTO_MSG_GETALG - CRYPTO_MSG_BASE] = { .doit = crypto_report,
.dump = crypto_dump_report,
- .done = crypto_dump_report_done},
+ .done = crypto_dump_report_done,
+ .module = THIS_MODULE},
};
static int crypto_user_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
@@ -471,6 +473,7 @@ static int crypto_user_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
.dump = link->dump,
.done = link->done,
.min_dump_alloc = dump_alloc,
+ .module = link->module,
};
return netlink_dump_start(crypto_nlsk, skb, nlh, &c);
}
--
1.7.7.6
^ permalink raw reply related
* [patch v3 07/11] nfnetlink_cttimeout: pass nfnetlink_cttimeout module to netlink_dump_start
From: Gao feng @ 2012-10-04 4:41 UTC (permalink / raw)
To: davem-fT/PcQaiUtIeIZ0/mPfg9Q, eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
steffen.klassert-opNxpl+3fjRBDgjK7y7TUQ
Cc: netfilter-devel-u79uwXL29TY76Z2rM5mHXA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-crypto-u79uwXL29TY76Z2rM5mHXA, pablo-Cap9r6Oaw4JrovVCs/uTlw,
stephen.hemminger-ZtmgI6mnKB3QT0dZR+AlfA, jengelh-9+2X+4sQBs8,
Gao feng
In-Reply-To: <1349325689-16104-1-git-send-email-gaofeng-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
set netlink_dump_control.module to avoid panic.
Signed-off-by: Gao feng <gaofeng-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
---
net/netfilter/nfnetlink_cttimeout.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/net/netfilter/nfnetlink_cttimeout.c b/net/netfilter/nfnetlink_cttimeout.c
index 8847b4d..e0ab57c 100644
--- a/net/netfilter/nfnetlink_cttimeout.c
+++ b/net/netfilter/nfnetlink_cttimeout.c
@@ -248,6 +248,7 @@ cttimeout_get_timeout(struct sock *ctnl, struct sk_buff *skb,
if (nlh->nlmsg_flags & NLM_F_DUMP) {
struct netlink_dump_control c = {
.dump = ctnl_timeout_dump,
+ .module = THIS_MODULE,
};
return netlink_dump_start(ctnl, skb, nlh, &c);
}
--
1.7.7.6
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [patch v3 06/11] nfnetlink_cthelper: pass nfnetlink_cthelper module to netlink_dump_start
From: Gao feng @ 2012-10-04 4:41 UTC (permalink / raw)
To: davem-fT/PcQaiUtIeIZ0/mPfg9Q, eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
steffen.klassert-opNxpl+3fjRBDgjK7y7TUQ
Cc: netfilter-devel-u79uwXL29TY76Z2rM5mHXA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-crypto-u79uwXL29TY76Z2rM5mHXA, pablo-Cap9r6Oaw4JrovVCs/uTlw,
stephen.hemminger-ZtmgI6mnKB3QT0dZR+AlfA, jengelh-9+2X+4sQBs8,
Gao feng
In-Reply-To: <1349325689-16104-1-git-send-email-gaofeng-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
set netlink_dump_control.module to avoid panic.
Signed-off-by: Gao feng <gaofeng-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
---
net/netfilter/nfnetlink_cthelper.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/net/netfilter/nfnetlink_cthelper.c b/net/netfilter/nfnetlink_cthelper.c
index 945950a..2bcd2d8 100644
--- a/net/netfilter/nfnetlink_cthelper.c
+++ b/net/netfilter/nfnetlink_cthelper.c
@@ -504,6 +504,7 @@ nfnl_cthelper_get(struct sock *nfnl, struct sk_buff *skb,
if (nlh->nlmsg_flags & NLM_F_DUMP) {
struct netlink_dump_control c = {
.dump = nfnl_cthelper_dump_table,
+ .module = THIS_MODULE,
};
return netlink_dump_start(nfnl, skb, nlh, &c);
}
--
1.7.7.6
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [patch v3 05/11] nfnetlink_acct: pass nfnetlink_acct module to netlink_dump_start
From: Gao feng @ 2012-10-04 4:41 UTC (permalink / raw)
To: davem, eric.dumazet, steffen.klassert
Cc: netfilter-devel, linux-rdma, netdev, linux-crypto, pablo,
stephen.hemminger, jengelh, Gao feng
In-Reply-To: <1349325689-16104-1-git-send-email-gaofeng@cn.fujitsu.com>
set netlink_dump_control.module to avoid panic.
Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
---
net/netfilter/nfnetlink_acct.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/net/netfilter/nfnetlink_acct.c b/net/netfilter/nfnetlink_acct.c
index 589d686..8acf8b8 100644
--- a/net/netfilter/nfnetlink_acct.c
+++ b/net/netfilter/nfnetlink_acct.c
@@ -175,6 +175,7 @@ nfnl_acct_get(struct sock *nfnl, struct sk_buff *skb,
if (nlh->nlmsg_flags & NLM_F_DUMP) {
struct netlink_dump_control c = {
.dump = nfnl_acct_dump,
+ .module = THIS_MODULE,
};
return netlink_dump_start(nfnl, skb, nlh, &c);
}
--
1.7.7.6
^ permalink raw reply related
* [patch v3 04/11] nf_conntrack_netlink: pass nf_conntrack_netlink module to netlink_dump_start
From: Gao feng @ 2012-10-04 4:41 UTC (permalink / raw)
To: davem, eric.dumazet, steffen.klassert
Cc: netfilter-devel, linux-rdma, netdev, linux-crypto, pablo,
stephen.hemminger, jengelh, Gao feng
In-Reply-To: <1349325689-16104-1-git-send-email-gaofeng@cn.fujitsu.com>
set netlink_dump_control.module to avoid panic.
Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
---
net/netfilter/nf_conntrack_netlink.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index 7bbfb3d..20af287 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -1022,6 +1022,7 @@ ctnetlink_get_conntrack(struct sock *ctnl, struct sk_buff *skb,
struct netlink_dump_control c = {
.dump = ctnetlink_dump_table,
.done = ctnetlink_done,
+ .module = THIS_MODULE,
};
#ifdef CONFIG_NF_CONNTRACK_MARK
if (cda[CTA_MARK] && cda[CTA_MARK_MASK]) {
@@ -1723,6 +1724,7 @@ ctnetlink_stat_ct_cpu(struct sock *ctnl, struct sk_buff *skb,
if (nlh->nlmsg_flags & NLM_F_DUMP) {
struct netlink_dump_control c = {
.dump = ctnetlink_ct_stat_cpu_dump,
+ .module = THIS_MODULE,
};
return netlink_dump_start(ctnl, skb, nlh, &c);
}
@@ -2242,6 +2244,7 @@ ctnetlink_get_expect(struct sock *ctnl, struct sk_buff *skb,
struct netlink_dump_control c = {
.dump = ctnetlink_exp_dump_table,
.done = ctnetlink_exp_done,
+ .module = THIS_MODULE,
};
return netlink_dump_start(ctnl, skb, nlh, &c);
}
@@ -2680,6 +2683,7 @@ ctnetlink_stat_exp_cpu(struct sock *ctnl, struct sk_buff *skb,
if (nlh->nlmsg_flags & NLM_F_DUMP) {
struct netlink_dump_control c = {
.dump = ctnetlink_exp_stat_cpu_dump,
+ .module = THIS_MODULE,
};
return netlink_dump_start(ctnl, skb, nlh, &c);
}
--
1.7.7.6
^ permalink raw reply related
* [patch v3 03/11] unix_diag: pass unix_diag module to netlink_dump_start
From: Gao feng @ 2012-10-04 4:41 UTC (permalink / raw)
To: davem-fT/PcQaiUtIeIZ0/mPfg9Q, eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
steffen.klassert-opNxpl+3fjRBDgjK7y7TUQ
Cc: netfilter-devel-u79uwXL29TY76Z2rM5mHXA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-crypto-u79uwXL29TY76Z2rM5mHXA, pablo-Cap9r6Oaw4JrovVCs/uTlw,
stephen.hemminger-ZtmgI6mnKB3QT0dZR+AlfA, jengelh-9+2X+4sQBs8,
Gao feng
In-Reply-To: <1349325689-16104-1-git-send-email-gaofeng-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
set netlink_dump_control.module to avoid panic.
Signed-off-by: Gao feng <gaofeng-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
---
net/unix/diag.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/net/unix/diag.c b/net/unix/diag.c
index 06748f1..e1cd31f 100644
--- a/net/unix/diag.c
+++ b/net/unix/diag.c
@@ -299,6 +299,7 @@ static int unix_diag_handler_dump(struct sk_buff *skb, struct nlmsghdr *h)
if (h->nlmsg_flags & NLM_F_DUMP) {
struct netlink_dump_control c = {
.dump = unix_diag_dump,
+ .module = THIS_MODULE,
};
return netlink_dump_start(net->diag_nlsk, skb, h, &c);
} else
--
1.7.7.6
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [patch v3 02/11] inet_diag: pass inet_diag module to netlink_dump_start
From: Gao feng @ 2012-10-04 4:41 UTC (permalink / raw)
To: davem, eric.dumazet, steffen.klassert
Cc: netfilter-devel, linux-rdma, netdev, linux-crypto, pablo,
stephen.hemminger, jengelh, Gao feng
In-Reply-To: <1349325689-16104-1-git-send-email-gaofeng@cn.fujitsu.com>
set netlink_dump_control.module to avoid panic.
Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
---
net/ipv4/inet_diag.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/net/ipv4/inet_diag.c b/net/ipv4/inet_diag.c
index 535584c..5ffd7bc 100644
--- a/net/ipv4/inet_diag.c
+++ b/net/ipv4/inet_diag.c
@@ -981,6 +981,7 @@ static int inet_diag_rcv_msg_compat(struct sk_buff *skb, struct nlmsghdr *nlh)
{
struct netlink_dump_control c = {
.dump = inet_diag_dump_compat,
+ .module = THIS_MODULE,
};
return netlink_dump_start(net->diag_nlsk, skb, nlh, &c);
}
@@ -1010,6 +1011,7 @@ static int inet_diag_handler_dump(struct sk_buff *skb, struct nlmsghdr *h)
{
struct netlink_dump_control c = {
.dump = inet_diag_dump,
+ .module = THIS_MODULE,
};
return netlink_dump_start(net->diag_nlsk, skb, h, &c);
}
--
1.7.7.6
^ permalink raw reply related
* [patch v3 01/11] netlink: add reference of module in netlink_dump_start
From: Gao feng @ 2012-10-04 4:41 UTC (permalink / raw)
To: davem, eric.dumazet, steffen.klassert
Cc: netfilter-devel, linux-rdma, netdev, linux-crypto, pablo,
stephen.hemminger, jengelh, Gao feng
I get a panic when I use ss -a and rmmod inet_diag at the
same time.
it's because netlink_dump use inet_diag_dump witch function
belongs to module inet_diag.
I search the codes and find many modules have the same problem.
We need add reference of the module witch the cb->dump belongs
to.
Thanks for all help from Stephen,Jan,Eric,Steffen and Pablo.
Change From v2:
delete netlink_dump_done,and call module_put in netlink_dump
and netlink_sock_destruct.
Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
---
include/linux/netlink.h | 5 ++++-
net/netlink/af_netlink.c | 21 +++++++++++++++++----
2 files changed, 21 insertions(+), 5 deletions(-)
diff --git a/include/linux/netlink.h b/include/linux/netlink.h
index f80c56a..bf4e501 100644
--- a/include/linux/netlink.h
+++ b/include/linux/netlink.h
@@ -245,6 +245,8 @@ struct netlink_callback {
struct netlink_callback *cb);
int (*done)(struct netlink_callback *cb);
void *data;
+ /* the module that dump function belong to */
+ struct module *module;
u16 family;
u16 min_dump_alloc;
unsigned int prev_seq, seq;
@@ -262,8 +264,9 @@ __nlmsg_put(struct sk_buff *skb, u32 portid, u32 seq, int type, int len, int fla
struct netlink_dump_control {
int (*dump)(struct sk_buff *skb, struct netlink_callback *);
- int (*done)(struct netlink_callback*);
+ int (*done)(struct netlink_callback *);
void *data;
+ struct module *module;
u16 min_dump_alloc;
};
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 0f2e3ad..0905dfb 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -169,6 +169,7 @@ static void netlink_sock_destruct(struct sock *sk)
if (nlk->cb) {
if (nlk->cb->done)
nlk->cb->done(nlk->cb);
+ module_put(nlk->cb->module);
netlink_destroy_callback(nlk->cb);
}
@@ -1755,6 +1756,8 @@ static int netlink_dump(struct sock *sk)
if (cb->done)
cb->done(cb);
+
+ module_put(cb->module);
nlk->cb = NULL;
mutex_unlock(nlk->cb_mutex);
@@ -1784,6 +1787,7 @@ int netlink_dump_start(struct sock *ssk, struct sk_buff *skb,
cb->done = control->done;
cb->nlh = nlh;
cb->data = control->data;
+ cb->module = control->module;
cb->min_dump_alloc = control->min_dump_alloc;
atomic_inc(&skb->users);
cb->skb = skb;
@@ -1794,19 +1798,28 @@ int netlink_dump_start(struct sock *ssk, struct sk_buff *skb,
return -ECONNREFUSED;
}
nlk = nlk_sk(sk);
- /* A dump is in progress... */
+
mutex_lock(nlk->cb_mutex);
+ /* A dump is in progress... */
if (nlk->cb) {
mutex_unlock(nlk->cb_mutex);
netlink_destroy_callback(cb);
- sock_put(sk);
- return -EBUSY;
+ ret = -EBUSY;
+ goto out;
}
+ /* add reference of module witch cb->dump belong to */
+ if (!try_module_get(cb->module)) {
+ mutex_unlock(nlk->cb_mutex);
+ netlink_destroy_callback(cb);
+ ret = -EPROTONOSUPPORT;
+ goto out;
+ }
+
nlk->cb = cb;
mutex_unlock(nlk->cb_mutex);
ret = netlink_dump(sk);
-
+out:
sock_put(sk);
if (ret)
--
1.7.7.6
^ permalink raw reply related
* [RFC] vxlan: use ether header as fallback hash
From: Stephen Hemminger @ 2012-10-04 4:39 UTC (permalink / raw)
To: Jesse Gross, davem, netdev
In-Reply-To: <CAEP_g=9Y3MmsABQJyi508FEgs4635tnaVqk4Ncwbpm41bLskhg@mail.gmail.com>
VXLAN bases source UDP port based on flow to help the
receiver to be able to load balance based on outer header
contents.
This patches changes the algorithm to better handle packets
that can not be categorized by the rxhash() function.
It adds a fallback to use jhash on the Ether header.
It also fixes a bug where the old code could assign 0 as a port
value.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
---
RFC for now, compile tested only
--- a/drivers/net/vxlan.c 2012-10-03 21:25:43.747968165 -0700
+++ b/drivers/net/vxlan.c 2012-10-03 21:36:10.213805422 -0700
@@ -622,12 +622,30 @@ static inline u8 vxlan_ecn_encap(u8 tos,
return INET_ECN_encapsulate(tos, inner);
}
+/* Compute hash to use for source port
+ * first choice to use L4 flow hash since it will spread
+ * better and maybe available from hardware
+ * secondary choice is to use jhash on the Ethernet header
+ * Always returns non-zero value
+ */
+static u16 vxlan_flow_hash(struct sk_buff *skb)
+{
+ u16 hash = skb_get_rxhash(skb);
+
+ if (!hash)
+ hash = jhash(skb->data, 3, skb->protocol);
+
+ if (!hash)
+ hash = 1;
+
+ return hash;
+}
+
/* Transmit local packets over Vxlan
*
* Outer IP header inherits ECN and DF from inner header.
* Outer UDP destination is the VXLAN assigned port.
- * source port is based on hash of flow if available
- * otherwise use a random value
+ * source port is based on hash of flow
*/
static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct net_device *dev)
{
@@ -641,8 +659,8 @@ static netdev_tx_t vxlan_xmit(struct sk_
struct flowi4 fl4;
struct vxlan_fdb *f;
unsigned int pkt_len = skb->len;
- u32 hash;
__be32 dst;
+ __be16 src_port;
__be16 df = 0;
__u8 tos, ttl;
int err;
@@ -670,7 +688,7 @@ static netdev_tx_t vxlan_xmit(struct sk_
if (tos == 1)
tos = vxlan_get_dsfield(old_iph, skb);
- hash = skb_get_rxhash(skb);
+ src_port = (__force __be16) vxlan_flow_hash(skb);
rt = ip_route_output_gre(dev_net(dev), &fl4, dst,
vxlan->saddr, vxlan->vni,
@@ -703,7 +721,7 @@ static netdev_tx_t vxlan_xmit(struct sk_
uh = udp_hdr(skb);
uh->dest = htons(vxlan_port);
- uh->source = hash ? :random32();
+ uh->source = src_port;
uh->len = htons(skb->len);
uh->check = 0;
^ permalink raw reply
* Re: [PATCH 0/3] virtio-net: inline header support
From: Anthony Liguori @ 2012-10-04 4:29 UTC (permalink / raw)
To: Rusty Russell, Michael S. Tsirkin, Thomas Lendacky
Cc: Sasha Levin, virtualization, linux-kernel, avi, kvm, netdev
In-Reply-To: <87391u3o67.fsf@rustcorp.com.au>
Rusty Russell <rusty@rustcorp.com.au> writes:
> Anthony Liguori <anthony@codemonkey.ws> writes:
>> Rusty Russell <rusty@rustcorp.com.au> writes:
>>
>>> "Michael S. Tsirkin" <mst@redhat.com> writes:
>>>
>>>> Thinking about Sasha's patches, we can reduce ring usage
>>>> for virtio net small packets dramatically if we put
>>>> virtio net header inline with the data.
>>>> This can be done for free in case guest net stack allocated
>>>> extra head room for the packet, and I don't see
>>>> why would this have any downsides.
>>>
>>> I've been wanting to do this for the longest time... but...
>>>
>>>> Even though with my recent patches qemu
>>>> no longer requires header to be the first s/g element,
>>>> we need a new feature bit to detect this.
>>>> A trivial qemu patch will be sent separately.
>>>
>>> There's a reason I haven't done this. I really, really dislike "my
>>> implemention isn't broken" feature bits. We could have an infinite
>>> number of them, for each bug in each device.
>>
>> This is a bug in the specification.
>>
>> The QEMU implementation pre-dates the specification. All of the actual
>> implementations of virtio relied on the semantics of s/g elements and
>> still do.
>
> lguest fix is pending in my queue. lkvm and qemu are broken; lkvm isn't
> ever going to be merged, so I'm not sure what its status is? But I'm
> determined to fix qemu, and hence my torture patch to make sure this
> doesn't creep in again.
There are even more implementations out there and I'd wager they all
rely on framing.
>> What's in the specification really doesn't matter when it doesn't agree
>> with all of the existing implementations.
>>
>> Users use implementations, not specifications. The specification really
>> ought to be changed here.
>
> I'm sorely tempted, except that we're losing a real optimization because
> of this :(
What optimizations? What Michael is proposing is still achievable with
a device feature. Are there other optimizations that can be achieved by
changing framing that we can't achieve with feature bits?
As I mentioned in another note, bad framing decisions can cause
performance issues too...
> The specification has long contained the footnote:
>
> The current qemu device implementations mistakenly insist that
> the first descriptor cover the header in these cases exactly, so
> a cautious driver should arrange it so.
I seem to recall this being a compromise between you and I.. I think
I objected strongly to this back when you first wrote the spec and you
added this to appease me ;-)
Regards,
Anthony Liguori
>
> I'd like to tie this caveat to the PCI capability change, so this note
> will move to the appendix with the old PCI layout.
>
> Cheers,
> Rusty.
^ permalink raw reply
* RE: [PATCHv4 1/4] modem_shm: Add Modem Access Framework
From: Arun MURTHY @ 2012-10-04 4:08 UTC (permalink / raw)
To: Greg KH
Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
linux-doc@vger.kernel.org, alan@lxorguk.ukuu.org.uk
In-Reply-To: <20121003151748.GA5745@kroah.com>
> On Wed, Oct 03, 2012 at 05:54:08AM +0200, Arun MURTHY wrote:
> > > On Mon, Oct 01, 2012 at 07:30:38AM +0200, Arun MURTHY wrote:
> > > > > On Fri, Sep 28, 2012 at 01:35:01PM +0530, Arun Murthy wrote:
> > > > > > +#include <linux/module.h>
> > > > > > +#include <linux/slab.h>
> > > > > > +#include <linux/err.h>
> > > > > > +#include <linux/printk.h>
> > > > > > +#include <linux/modem_shm/modem.h>
> > > > > > +
> > > > > > +static struct class *modem_class;
> > > > >
> > > > > What's wrong with a bus_type instead?
> > > >
> > > > Can I know the advantage of using bus_type over class?
> > >
> > > You have devices living on a bus, and it's much more descriptive
> > > than a class (which we are going to eventually get rid of one of these
> days...).
> > >
> > > Might I ask why you choose a class over a bus_type?
> >
> > Basically my requirement is to create a central entity for accessing
> > and releasing modem from APE.
>
> What is an "APE"?
>
> And what do you mean by "accessing" and "releasing"?
APE - Application Processor Engine
There are two processors but on a single chip, one being the APE and other
is the modem. So 'accessing' means requesting access or waking-up the
co-processor and releasing means allowing the co-processor to sleep.
>
> > Since this is done by different clients the central entity should be
> > able to handle the request and play safely, since this has more affect
> > in system suspend and deep sleep. Using class helps me in achieving
> > this and also create an entry to user space which can be used in the
> > later parts. Moreover this not something like a bus or so, so I didn't
> > use bus instead went with a simple class approach.
>
> But as you have devices that are "binding" to this "controller", a bus might
> make more sense, right?
Have explained above regarding the platform, the concept of bus doesn't
come into picture at all. Here its just waking-up the modem and allowing
it to go to sleep.
>
> I don't see how a class helps out for you here more than anything else, what
> are you expecting from the class interface? You aren't using the reference
> counting logic it provides, so why use it at all?
I am using the reference counting logic in class such as class_for_each_device.
>
> Actually, why use the driver core at all in the first place if you aren't needing
> the devices to show up in sysfs (as you don't have a device, you are just a
> mediator)?
Yes I am something like a mediator, but since this is associated with many
clients, there should be some central entity to take inputs from all the clients
and act accordingly. This MAF does that. Sysfs will also be created for this
MAF in the coming versions.
>
> > > > > > +int modem_release(struct modem_desc *mdesc) {
> > > > > > + if (!mdesc->release)
> > > > > > + return -EFAULT;
> > > > > > +
> > > > > > + if (modem_is_requested(mdesc)) {
> > > > > > + atomic_dec(&mdesc->mclients->cnt);
> > > > > > + if (atomic_read(&mdesc->use_cnt) == 1) {
> > > > > > + mdesc->release(mdesc);
> > > > > > + atomic_dec(&mdesc->use_cnt);
> > > > > > + }
> > > > >
> > > > > Eeek, why aren't you using the built-in reference counting that
> > > > > the struct device provided to you, and instead are rolling your own?
> > > > > This happens in many places, why?
> > > >
> > > > My usage of counters over here is for each modem there are many
> clients.
> > > > Each of the clients will have a ref to modem_desc. Each of them
> > > > use this for requesting and releasing the modem. One counter for
> > > > tracking the request and release for each client which is done by
> > > > variable 'cnt' in
> > > struct clients.
> > > > The counter use_cnt is used for tracking the modem request/release
> > > > irrespective of the clients and counter cli_cnt is used for
> > > > restricting the modem_get to the no of clients defined in no_clients.
> > > >
> > > > So totally 3 counter one for restricting the usage of modem_get by
> > > > clients, second for restricting modem request/release at top
> > > > level, and 3rd for restricting modem release/request for per
> > > > client per modem
> > > basis.
> > > >
> > > > Can you let me know if the same can be achieved by using built-in
> > > > ref counting?
> > >
> > > Yes, because you don't need all of those different levels, just
> > > stick with one and you should be fine. :)
> > >
> >
> > No, checks at all these levels are required, I have briefed out the need also.
>
> I still don't understand, sorry.
The pictorial view by Anish should help in understanding.
Modem Client1 Client2 Client3 Client4
State turn-on request
State no-state-change request
State no-state-change request
State no-state-change request
State no-state-change release
State no-state-change release
State no-state-change release
State turn-off release
This is just a simple straight forward example.
>
> > This will have effect on system power management, i.e suspend and deep
> > sleep.
>
> How does power management matter? If you tie into the driver model
> properly, power management comes "for free" so you don't have to do
> anything special about it. Why not use that logic instead of trying to roll your
> own?
As said there are two processors on a single chip playing over here. One being
the APE(Application Processor Engine) and other is Modem. Since they are on
a single chip but for APE entering into deep sleep modem should be released.
>
> > We restrict that the drivers should request modem only once and
> > release only once, but we cannot rely on the clients hence a check for
> > the same has to be done in the MAF.
>
> You can't rely on the clients to do what? And why can't you rely on them?
> What is going to happen? Who is a "client" here? Other kernel code?
Yes, let me take my driver itself as an example. Here the clients are the
shared memory driver, sim driver, security etc. Shared memory driver
is the communicating media between the APE and Modem and hence
needs to wake-up the modem and after completion should allow modem
to enter sleep.
Similarly it's the same for sim driver also.
We define that the clients such as shared memory driver and the sim
driver should request for modem only one and then release only once
and also since this is a MAF shouldn't it take care of checking the same?
>
> I really don't understand your model at all as to what you are trying to
> mediate and manage here, sorry. I suggest writing it all up as your first patch
> (documentation is good), so that we can properly review your
> implementation and not argue about how to implement something that I
> honestly don't understand.
Sorry for that. Actually my 4th patch in this patchset includes the documentation.
Since it's the kernel doc I have made it as the last patch in this patchset, else
kernel doc compilation will fail.
Please feel free to refer the 4th patch for the documentation part and if still
not clear I can provide more explanation on this.
>
> > Also the no of clients should be defined and hence a check for the
> > same is done in MAF.
>
> Defined where? What is "MAF"?
This driver is MAF(Modem Access Framework).
>
> > Apart from all these the requests coming from all the clients is to be
> > accumulated and based on that modem release or access should be
> > performed, hence so.
>
> That sentance makes no sense to me, it must be too early for me here...
Thanks and Regards,
Arun R Murthy
-----------------
^ permalink raw reply
* Re: [PATCH 3/3] vxlan: virtual extensible lan
From: Stephen Hemminger @ 2012-10-04 4:02 UTC (permalink / raw)
To: Jesse Gross; +Cc: davem, netdev
In-Reply-To: <CAEP_g=9Y3MmsABQJyi508FEgs4635tnaVqk4Ncwbpm41bLskhg@mail.gmail.com>
On Wed, 3 Oct 2012 18:54:46 -0700
Jesse Gross <jesse@nicira.com> wrote:
> On Mon, Oct 1, 2012 at 3:32 PM, Stephen Hemminger <shemminger@vyatta.com> wrote:
> > --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> > +++ b/drivers/net/vxlan.c 2012-10-01 15:08:38.024499080 -0700
> > +/* Transmit local packets over Vxlan
> > + *
> > + * Outer IP header inherits ECN and DF from inner header.
> > + * Outer UDP destination is the VXLAN assigned port.
> > + * source port is based on hash of flow if available
> > + * otherwise use a random value
> > + */
> > +static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct net_device *dev)
> [...]
> > + hash = skb_get_rxhash(skb);
> [...]
> > + uh->dest = htons(vxlan_port);
> > + uh->source = hash ? :random32();
>
> I think this may lead to packet reordering in some cases. If the
> protocol is unknown then it may still represent part of a flow and
> random source ports could lead to taking different paths.
>
> Also, does it make sense to restrict the range of ports to, say, the
> upper half of the range?
Since port is always a pair, there is no need to restrict range, unless
there was a broken firewall in the way. One bug there is that random32()
can return 0 which is not a valid port number. A better fallback
would be a hash of the MAC header.
^ permalink raw reply
* Re: [PATCH 0/3] virtio-net: inline header support
From: Rusty Russell @ 2012-10-04 3:34 UTC (permalink / raw)
To: Anthony Liguori, Michael S. Tsirkin, Thomas Lendacky
Cc: Sasha Levin, virtualization, linux-kernel, avi, kvm, netdev
In-Reply-To: <87mx033u74.fsf@codemonkey.ws>
Anthony Liguori <anthony@codemonkey.ws> writes:
> Rusty Russell <rusty@rustcorp.com.au> writes:
>
>> "Michael S. Tsirkin" <mst@redhat.com> writes:
>>
>>> Thinking about Sasha's patches, we can reduce ring usage
>>> for virtio net small packets dramatically if we put
>>> virtio net header inline with the data.
>>> This can be done for free in case guest net stack allocated
>>> extra head room for the packet, and I don't see
>>> why would this have any downsides.
>>
>> I've been wanting to do this for the longest time... but...
>>
>>> Even though with my recent patches qemu
>>> no longer requires header to be the first s/g element,
>>> we need a new feature bit to detect this.
>>> A trivial qemu patch will be sent separately.
>>
>> There's a reason I haven't done this. I really, really dislike "my
>> implemention isn't broken" feature bits. We could have an infinite
>> number of them, for each bug in each device.
>
> This is a bug in the specification.
>
> The QEMU implementation pre-dates the specification. All of the actual
> implementations of virtio relied on the semantics of s/g elements and
> still do.
lguest fix is pending in my queue. lkvm and qemu are broken; lkvm isn't
ever going to be merged, so I'm not sure what its status is? But I'm
determined to fix qemu, and hence my torture patch to make sure this
doesn't creep in again.
> What's in the specification really doesn't matter when it doesn't agree
> with all of the existing implementations.
>
> Users use implementations, not specifications. The specification really
> ought to be changed here.
I'm sorely tempted, except that we're losing a real optimization because
of this :(
The specification has long contained the footnote:
The current qemu device implementations mistakenly insist that
the first descriptor cover the header in these cases exactly, so
a cautious driver should arrange it so.
I'd like to tie this caveat to the PCI capability change, so this note
will move to the appendix with the old PCI layout.
Cheers,
Rusty.
^ permalink raw reply
* Re: [PATCH] ipv6: release referenct of ip6_null_entry's dst entry in __ip6_del_rt
From: David Miller @ 2012-10-04 3:02 UTC (permalink / raw)
To: gaofeng; +Cc: netdev
In-Reply-To: <506CFA96.4050207@cn.fujitsu.com>
From: Gao feng <gaofeng@cn.fujitsu.com>
Date: Thu, 04 Oct 2012 10:55:18 +0800
> 于 2012年09月24日 12:32, Gao feng 写道:
>> 于 2012年09月22日 01:16, David Miller 写道:
>>> From: Gao feng <gaofeng@cn.fujitsu.com>
>>> Date: Thu, 20 Sep 2012 13:25:34 +0800
>>>
>>>> as we hold dst_entry before we call __ip6_del_rt,
>>>> so we should alse call dst_release not only return
>>>> -ENOENT when the rt6_info is ip6_null_entry.
>>>>
>>>> and we already hold the dst entry, so I think it's
>>>> safe to call dst_release out of the write-read lock.
>>>>
>>>> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
...
> Can you apply this patch?
Sorry, I've been really busy this week. I will try to get to
this soon.
Thanks.
^ permalink raw reply
* Re: [PATCH] ipv6: release referenct of ip6_null_entry's dst entry in __ip6_del_rt
From: Gao feng @ 2012-10-04 2:55 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <505FE24E.4030204@cn.fujitsu.com>
于 2012年09月24日 12:32, Gao feng 写道:
> 于 2012年09月22日 01:16, David Miller 写道:
>> From: Gao feng <gaofeng@cn.fujitsu.com>
>> Date: Thu, 20 Sep 2012 13:25:34 +0800
>>
>>> as we hold dst_entry before we call __ip6_del_rt,
>>> so we should alse call dst_release not only return
>>> -ENOENT when the rt6_info is ip6_null_entry.
>>>
>>> and we already hold the dst entry, so I think it's
>>> safe to call dst_release out of the write-read lock.
>>>
>>> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
>>
>> I cannot find a code path where we can actually end up
>> with an ip6_null_entry here and I'd much rather declare
>> this condition as a bug.
>>
>> Patrick McHardy added this check back in 2006:
>>
>> commit 6c813a7297e3af4cd7c3458e09e9ee3d161c6830
>> Author: Patrick McHardy <kaber@trash.net>
>> Date: Sun Aug 6 22:22:47 2006 -0700
>>
>> [IPV6]: Fix crash in ip6_del_rt
>>
>> But the ipv6 code has changed substantially since then.
>
> ip -6 route del ::/0 will end up with an ip6_null_entry.
> so I think this checking and this patch is needed .
>
Hi David
Can you apply this patch?
thanks!
^ permalink raw reply
* RE: linux-next: build failure after merge of the net tree
From: Jay Hernandez @ 2012-10-04 2:12 UTC (permalink / raw)
To: David Miller, sfr
Cc: netdev, linux-next, linux-kernel, Vipul Pandya, Jay Hernandez
In-Reply-To: <20121003.214750.2022020970468412510.davem@davemloft.net>
David and Stephen,
I apologize for not taking the necessary steps to verify our patches
before submission. Ignorance is not an excuse... Anyways David thank you
for fixing the issue for us.
Jay-
-----Original Message-----
From: David Miller [mailto:davem@davemloft.net]
Sent: Wednesday, October 03, 2012 6:48 PM
To: sfr@canb.auug.org.au
Cc: netdev@vger.kernel.org; linux-next@vger.kernel.org;
linux-kernel@vger.kernel.org; Vipul Pandya; Jay Hernandez
Subject: Re: linux-next: build failure after merge of the net tree
From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Thu, 4 Oct 2012 11:06:15 +1000
> Actually, for me it is. I have a script that does the "use
> yesterday's version" for me. To fix (even a one liner) means bringing
> up an editor, commiting, creating the patch and then recommiting it
> (an implementation
> detail) and recording that I need to keep (automatically) applying the
> patch in case the maintainer doesn't react quickly.
Ok, fair enough.
I'll try to look into making x86 barf when a vmalloc.h include is
missing.
^ permalink raw reply
* RE: Possible bug with r8169 driver
From: hayeswang @ 2012-10-04 2:09 UTC (permalink / raw)
To: 'Francois Romieu'; +Cc: 'Nolwenn', netdev, 'nic_swsd'
In-Reply-To: <20121001185629.GA4922@electric-eye.fr.zoreil.com>
Francois Romieu [mailto:romieu@fr.zoreil.com]
[...]
> diff --git a/drivers/net/ethernet/realtek/r8169.c
> b/drivers/net/ethernet/realtek/r8169.c
> index eb81da4..e0f1b8d 100644
> --- a/drivers/net/ethernet/realtek/r8169.c
> +++ b/drivers/net/ethernet/realtek/r8169.c
> @@ -4271,8 +4271,8 @@ static void rtl_set_rx_mode(struct
> net_device *dev)
> mc_filter[1] = swab32(data);
> }
>
> - RTL_W32(MAR0 + 4, mc_filter[1]);
> - RTL_W32(MAR0 + 0, mc_filter[0]);
> + RTL_W32(MAR0 + 4, 0xffffffff);
> + RTL_W32(MAR0 + 0, 0xffffffff);
>
> RTL_W32(RxConfig, tmp);
> }
The RTL_GIGA_MAC_VER_35 is designed without multicast hardware filter, so you
should set IO 0x08 ~ 0x0f to 0xff for receiving all multicast packets without
filtering them.
Best Regards,
Hayes
^ permalink raw reply
* Re: [PATCH 3/3] vxlan: virtual extensible lan
From: Jesse Gross @ 2012-10-04 1:54 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: davem, netdev
In-Reply-To: <20121001223254.349753999@vyatta.com>
On Mon, Oct 1, 2012 at 3:32 PM, Stephen Hemminger <shemminger@vyatta.com> wrote:
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ b/drivers/net/vxlan.c 2012-10-01 15:08:38.024499080 -0700
> +/* Transmit local packets over Vxlan
> + *
> + * Outer IP header inherits ECN and DF from inner header.
> + * Outer UDP destination is the VXLAN assigned port.
> + * source port is based on hash of flow if available
> + * otherwise use a random value
> + */
> +static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct net_device *dev)
[...]
> + hash = skb_get_rxhash(skb);
[...]
> + uh->dest = htons(vxlan_port);
> + uh->source = hash ? :random32();
I think this may lead to packet reordering in some cases. If the
protocol is unknown then it may still represent part of a flow and
random source ports could lead to taking different paths.
Also, does it make sense to restrict the range of ports to, say, the
upper half of the range?
^ permalink raw reply
* Re: linux-next: build failure after merge of the net tree
From: David Miller @ 2012-10-04 1:47 UTC (permalink / raw)
To: sfr; +Cc: netdev, linux-next, linux-kernel, vipul, jay
In-Reply-To: <20121004110615.3fda3c25735284c776106d7d@canb.auug.org.au>
From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Thu, 4 Oct 2012 11:06:15 +1000
> Actually, for me it is. I have a script that does the "use yesterday's
> version" for me. To fix (even a one liner) means bringing up an editor,
> commiting, creating the patch and then recommiting it (an implementation
> detail) and recording that I need to keep (automatically) applying the
> patch in case the maintainer doesn't react quickly.
Ok, fair enough.
I'll try to look into making x86 barf when a vmalloc.h include is
missing.
^ permalink raw reply
* Re: [PATCH 0/3] virtio-net: inline header support
From: Anthony Liguori @ 2012-10-04 1:35 UTC (permalink / raw)
To: Rusty Russell, Michael S. Tsirkin, Thomas Lendacky
Cc: Sasha Levin, virtualization, linux-kernel, avi, kvm, netdev
In-Reply-To: <87vces2gxq.fsf@rustcorp.com.au>
Rusty Russell <rusty@rustcorp.com.au> writes:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
>
> There's a reason I haven't done this. I really, really dislike "my
> implemention isn't broken" feature bits. We could have an infinite
> number of them, for each bug in each device.
>
> So my plan was to tie this assumption to the new PCI layout. And have a
> stress-testing patch like the one below in the kernel (see my virtio-wip
> branch for stuff like this). Turn it on at boot with
> "virtio_ring.torture" on the kernel commandline.
>
> BTW, I've fixed lguest, but my kvm here (Ubuntu precise, kvm-qemu 1.0)
> is too old. Building the latest git now...
>
> Cheers,
> Rusty.
>
> Subject: virtio: CONFIG_VIRTIO_DEVICE_TORTURE
>
> Virtio devices are not supposed to depend on the framing of the scatter-gather
> lists, but various implementations did. Safeguard this in future by adding
> an option to deliberately create perverse descriptors.
>
> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Ignore framing is really a bad idea. You want backends to enforce
reasonable framing because guest's shouldn't do silly things with framing.
For instance, with virtio-blk, if you want decent performance, you
absolutely want to avoid bouncing the data. If you're using O_DIRECT in
the host to submit I/O requests, then it's critical that all of the s/g
elements are aligned to a sector boundary and sized to a sector
boundary.
Yes, QEMU can handle if that's not the case, but it would be insanely
stupid for a guest not to do this. This is the sort of thing that ought
to be enforced in the specification because a guest cannot perform well
if it doesn't follow these rules.
A spec isn't terribly useful if the result is guest drivers that are
slow. There's very little to gain by not enforcing rules around framing
and there's a lot to lose if a guest frames incorrectly.
In the rare case where we want to make a framing change, we should use
feature bits like Michael is proposing.
In this case, we should simply say that with the feature bit, the vnet
header can be in the same element as the data but not allow the header
to be spread across multiple elements.
Regards,
Anthony Liguori
>
> diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig
> index 8d5bddb..930a4ea 100644
> --- a/drivers/virtio/Kconfig
> +++ b/drivers/virtio/Kconfig
> @@ -5,6 +5,15 @@ config VIRTIO
> bus, such as CONFIG_VIRTIO_PCI, CONFIG_VIRTIO_MMIO, CONFIG_LGUEST,
> CONFIG_RPMSG or CONFIG_S390_GUEST.
>
> +config VIRTIO_DEVICE_TORTURE
> + bool "Virtio device torture tests"
> + depends on VIRTIO && DEBUG_KERNEL
> + help
> + This makes the virtio_ring implementation creatively change
> + the format of requests to make sure that devices are
> + properly implemented. This will make your virtual machine
> + slow *and* unreliable! Say N.
> +
> menu "Virtio drivers"
>
> config VIRTIO_PCI
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index e639584..8893753 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -124,6 +124,149 @@ struct vring_virtqueue
>
> #define to_vvq(_vq) container_of(_vq, struct vring_virtqueue, vq)
>
> +#ifdef CONFIG_VIRTIO_DEVICE_TORTURE
> +static bool torture;
> +module_param(torture, bool, 0644);
> +
> +struct torture {
> + unsigned int orig_out, orig_in;
> + void *orig_data;
> + struct scatterlist sg[4];
> + struct scatterlist orig_sg[];
> +};
> +
> +static size_t tot_len(struct scatterlist sg[], unsigned num)
> +{
> + size_t len, i;
> +
> + for (len = 0, i = 0; i < num; i++)
> + len += sg[i].length;
> +
> + return len;
> +}
> +
> +static void copy_sg_data(const struct scatterlist *dst, unsigned dnum,
> + const struct scatterlist *src, unsigned snum)
> +{
> + unsigned len;
> + struct scatterlist s, d;
> +
> + s = *src;
> + d = *dst;
> +
> + while (snum && dnum) {
> + len = min(s.length, d.length);
> + memcpy(sg_virt(&d), sg_virt(&s), len);
> + d.offset += len;
> + d.length -= len;
> + s.offset += len;
> + s.length -= len;
> + if (!s.length) {
> + BUG_ON(snum == 0);
> + src++;
> + snum--;
> + s = *src;
> + }
> + if (!d.length) {
> + BUG_ON(dnum == 0);
> + dst++;
> + dnum--;
> + d = *dst;
> + }
> + }
> +}
> +
> +static bool torture_replace(struct scatterlist **sg,
> + unsigned int *out,
> + unsigned int *in,
> + void **data,
> + gfp_t gfp)
> +{
> + static size_t seed;
> + struct torture *t;
> + size_t outlen, inlen, ourseed, len1;
> + void *buf;
> +
> + if (!torture)
> + return true;
> +
> + outlen = tot_len(*sg, *out);
> + inlen = tot_len(*sg + *out, *in);
> +
> + /* This will break horribly on large block requests. */
> + t = kmalloc(sizeof(*t) + (*out + *in) * sizeof(t->orig_sg[1])
> + + outlen + 1 + inlen + 1, gfp);
> + if (!t)
> + return false;
> +
> + sg_init_table(t->sg, 4);
> + buf = &t->orig_sg[*out + *in];
> +
> + memcpy(t->orig_sg, *sg, sizeof(**sg) * (*out + *in));
> + t->orig_out = *out;
> + t->orig_in = *in;
> + t->orig_data = *data;
> + *data = t;
> +
> + ourseed = ACCESS_ONCE(seed);
> + seed++;
> +
> + *sg = t->sg;
> + if (outlen) {
> + /* Split outbuf into two parts, one byte apart. */
> + *out = 2;
> + len1 = ourseed % (outlen + 1);
> + sg_set_buf(&t->sg[0], buf, len1);
> + buf += len1 + 1;
> + sg_set_buf(&t->sg[1], buf, outlen - len1);
> + buf += outlen - len1;
> + copy_sg_data(t->sg, *out, t->orig_sg, t->orig_out);
> + }
> +
> + if (inlen) {
> + /* Split inbuf into two parts, one byte apart. */
> + *in = 2;
> + len1 = ourseed % (inlen + 1);
> + sg_set_buf(&t->sg[*out], buf, len1);
> + buf += len1 + 1;
> + sg_set_buf(&t->sg[*out + 1], buf, inlen - len1);
> + buf += inlen - len1;
> + }
> + return true;
> +}
> +
> +static void *torture_done(struct torture *t)
> +{
> + void *data;
> +
> + if (!torture)
> + return t;
> +
> + if (t->orig_in)
> + copy_sg_data(t->orig_sg + t->orig_out, t->orig_in,
> + t->sg + (t->orig_out ? 2 : 0), 2);
> +
> + data = t->orig_data;
> + kfree(t);
> + return data;
> +}
> +
> +#else
> +static bool torture_replace(struct scatterlist **sg,
> + unsigned int *out,
> + unsigned int *in,
> + void **data,
> + gfp_t gfp)
> +{
> + return true;
> +}
> +
> +static void *torture_done(void *data)
> +{
> + return data;
> +}
> +#endif /* CONFIG_VIRTIO_DEVICE_TORTURE */
> +
> /* Set up an indirect table of descriptors and add it to the queue. */
> static int vring_add_indirect(struct vring_virtqueue *vq,
> struct scatterlist sg[],
> @@ -213,6 +356,9 @@ int virtqueue_add_buf(struct virtqueue *_vq,
>
> BUG_ON(data == NULL);
>
> + if (!torture_replace(&sg, &out, &in, &data, gfp))
> + return -ENOMEM;
> +
> #ifdef DEBUG
> {
> ktime_t now = ktime_get();
> @@ -246,6 +392,7 @@ int virtqueue_add_buf(struct virtqueue *_vq,
> if (out)
> vq->notify(&vq->vq);
> END_USE(vq);
> + torture_done(data);
> return -ENOSPC;
> }
>
> @@ -476,7 +623,7 @@ void *virtqueue_get_buf(struct virtqueue *_vq, unsigned int *len)
> #endif
>
> END_USE(vq);
> - return ret;
> + return torture_done(ret);
> }
> EXPORT_SYMBOL_GPL(virtqueue_get_buf);
>
> --
> 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 0/3] virtio-net: inline header support
From: Anthony Liguori @ 2012-10-04 1:24 UTC (permalink / raw)
To: Rusty Russell, Michael S. Tsirkin, Thomas Lendacky
Cc: Sasha Levin, virtualization, linux-kernel, avi, kvm, netdev
In-Reply-To: <87vces2gxq.fsf@rustcorp.com.au>
Rusty Russell <rusty@rustcorp.com.au> writes:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
>
>> Thinking about Sasha's patches, we can reduce ring usage
>> for virtio net small packets dramatically if we put
>> virtio net header inline with the data.
>> This can be done for free in case guest net stack allocated
>> extra head room for the packet, and I don't see
>> why would this have any downsides.
>
> I've been wanting to do this for the longest time... but...
>
>> Even though with my recent patches qemu
>> no longer requires header to be the first s/g element,
>> we need a new feature bit to detect this.
>> A trivial qemu patch will be sent separately.
>
> There's a reason I haven't done this. I really, really dislike "my
> implemention isn't broken" feature bits. We could have an infinite
> number of them, for each bug in each device.
This is a bug in the specification.
The QEMU implementation pre-dates the specification. All of the actual
implementations of virtio relied on the semantics of s/g elements and
still do.
What's in the specification really doesn't matter when it doesn't agree
with all of the existing implementations.
Users use implementations, not specifications. The specification really
ought to be changed here.
Regards,
Anthony Liguori
^ permalink raw reply
* Re: linux-next: build failure after merge of the net tree
From: Stephen Rothwell @ 2012-10-04 1:06 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-next, linux-kernel, vipul, jay
In-Reply-To: <20121003.205053.1711360628623711137.davem@davemloft.net>
[-- Attachment #1: Type: text/plain, Size: 1295 bytes --]
Hi David,
On Wed, 03 Oct 2012 20:50:53 -0400 (EDT) David Miller <davem@davemloft.net> wrote:
>
> I do have a question though, it is honestly really that much easier to
> revert a whole days worth of changes (and therefore not get the code
> tested at all) than to simply add the obvious one liner?
Actually, for me it is. I have a script that does the "use yesterday's
version" for me. To fix (even a one liner) means bringing up an editor,
commiting, creating the patch and then recommiting it (an implementation
detail) and recording that I need to keep (automatically) applying the
patch in case the maintainer doesn't react quickly.
In this particular case I have been telling people to include vmalloc.h
(and other things like slab.h) over and over for years ... its a pain
that x86 builds indirectly include so much stuff.
> It seems to me to be absolutely the wrong tradeoff in these situations.
I guess for the "current/fixes" tree during the merge window, you are
right. For the "normal" trees, does a delay of (usually) one day really
matter?
I used to fix all this stuff and it added considerably to the length of
my work day (which currently can be up to 16 hours long) :-(
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH 16/20] drivers/net/ethernet/renesas/sh_eth.c: fix error return code
From: Nobuhiro Iwamatsu @ 2012-10-04 0:52 UTC (permalink / raw)
To: Peter Senna Tschudin
Cc: davem, kernel-janitors, yoshihiro.shimoda.uh, netdev,
linux-kernel
In-Reply-To: <1349281090-10013-15-git-send-email-peter.senna@gmail.com>
Peter Senna Tschudin さんは書きました:
> From: Peter Senna Tschudin <peter.senna@gmail.com>
>
> Convert a nonnegative error return code to a negative one, as returned
> elsewhere in the function.
>
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
>
> // <smpl>
> (
> if@p1 (\(ret < 0\|ret != 0\))
> { ... return ret; }
> |
> ret@p1 = 0
> )
> ... when != ret = e1
> when != &ret
> *if(...)
> {
> ... when != ret = e2
> when forall
> return ret;
> }
> // </smpl>
>
> Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
Acked-by: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj@renesas.com>
^ permalink raw reply
* Re: linux-next: build failure after merge of the net tree
From: David Miller @ 2012-10-04 0:50 UTC (permalink / raw)
To: sfr; +Cc: netdev, linux-next, linux-kernel, vipul, jay
In-Reply-To: <20121004101833.019598b7e474c7b74e67ff20@canb.auug.org.au>
From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Thu, 4 Oct 2012 10:18:33 +1000
> After merging the net tree, today's linux-next build (powerpc
> ppc64_defconfig) failed like this:
>
> drivers/net/ethernet/chelsio/cxgb4/t4_hw.c: In function 't4_memory_rw':
> drivers/net/ethernet/chelsio/cxgb4/t4_hw.c:420:2: error: implicit declaration of function 'vmalloc' [-Werror=implicit-function-declaration]
> drivers/net/ethernet/chelsio/cxgb4/t4_hw.c:420:7: warning: assignment makes pointer from integer without a cast [enabled by default]
> drivers/net/ethernet/chelsio/cxgb4/t4_hw.c:487:2: error: implicit declaration of function 'vfree' [-Werror=implicit-function-declaration]
> drivers/net/ethernet/chelsio/cxgb4/t4_hw.c: In function 'get_vpd_params':
> drivers/net/ethernet/chelsio/cxgb4/t4_hw.c:530:6: warning: assignment makes pointer from integer without a cast [enabled by default]
>
> Caused by commit 8c357ebd5693 ("cxgb4: Dynamically allocate memory in
> t4_memory_rw() and get_vpd_params()"). Missing include of
> linux/vmalloc.h.
>
> I have used the net tree from next-20121003 for today.
Thanks, I'll fix this as is shown below.
I do have a question though, it is honestly really that much easier to
revert a whole days worth of changes (and therefore not get the code
tested at all) than to simply add the obvious one liner?
It seems to me to be absolutely the wrong tradeoff in these situations.
====================
[PATCH] cxgb4: Fix build error due to missing linux/vmalloc.h include.
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
drivers/net/ethernet/chelsio/cxgb4/cxgb4.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h b/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
index 745a1f5..31752b2 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
@@ -43,6 +43,7 @@
#include <linux/pci.h>
#include <linux/spinlock.h>
#include <linux/timer.h>
+#include <linux/vmalloc.h>
#include <asm/io.h>
#include "cxgb4_uld.h"
#include "t4_hw.h"
--
1.7.11.4
^ 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