* Re: [PATCH] net: Fix ehea warning when built in
From: David Miller @ 2013-11-21 18:07 UTC (permalink / raw)
To: mikey; +Cc: jeffm, ohering, jslaby, netdev, cascardo
In-Reply-To: <11586.1385008609@ale.ozlabs.ibm.com>
From: Michael Neuling <mikey@neuling.org>
Date: Thu, 21 Nov 2013 15:36:49 +1100
> In:
> commit 7a3a62128388a6af771e99df8628ddee2e8be7ca
> Author: Olaf Hering <ohering@suse.com>
> drivers/net/ethernet/ibm/ehea/ehea_main.c: add alias entry for portN properties
> We added ehea_module_device_table. Unfortunately we only use this when
> building as a module so we get the following warning when built in, like
> in pseries_defconfig:
>
> drivers/net/ethernet/ibm/ehea/ehea_main.c:105:1: warning: 'ehea_module_device_table' defined but not used [-Wunused-variable]
>
> This wraps ehea_module_device_table with #if MODULE.
>
> Signed-off-by: Michael Neuling <mikey@neuling.org>
I think putting an ugly MODULE ifdef into every driver that does something
like this is not going to cut it.
Rather, the MODULE_DEVICE_TABLE() definition when !MODULE should do something
reasonable, like make a void reference to the symbole.
I'm not applying this, the fix belongs in a generic place.
Thanks.
^ permalink raw reply
* [PATCH net-next 1/8] genl: Add genlmsg_new_unicast() for unicast message allocation
From: Thomas Graf @ 2013-11-21 18:13 UTC (permalink / raw)
To: jesse-l0M0P4e3n4LQT0dZR+AlfA, davem-fT/PcQaiUtIeIZ0/mPfg9Q
Cc: dev-yBygre7rU0TnMu66kgdUjQ, fleitner-H+wXaHxf7aLQT0dZR+AlfA,
eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA, bhutchings-s/n/eUQHGBpZroRs9YW3xA
In-Reply-To: <cover.1385057355.git.tgraf-G/eBtMaohhA@public.gmane.org>
Allocates a new sk_buff large enough to cover the specified payload
plus required Netlink headers. Will check receiving socket for
memory mapped i/o capability and use it if enabled. Will fall back
to non-mapped skb if message size exceeds the frame size of the ring.
Signed-of-by: Thomas Graf <tgraf-G/eBtMaohhA@public.gmane.org>
Reviewed-by: Daniel Borkmann <dborkman-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
include/net/genetlink.h | 4 ++++
net/netlink/genetlink.c | 21 +++++++++++++++++++++
2 files changed, 25 insertions(+)
diff --git a/include/net/genetlink.h b/include/net/genetlink.h
index ace4abf..1d50c6f 100644
--- a/include/net/genetlink.h
+++ b/include/net/genetlink.h
@@ -73,6 +73,7 @@ struct genl_family {
* @attrs: netlink attributes
* @_net: network namespace
* @user_ptr: user pointers
+ * @dst_sk: destination socket
*/
struct genl_info {
u32 snd_seq;
@@ -85,6 +86,7 @@ struct genl_info {
struct net * _net;
#endif
void * user_ptr[2];
+ struct sock * dst_sk;
};
static inline struct net *genl_info_net(struct genl_info *info)
@@ -177,6 +179,8 @@ void genl_notify(struct genl_family *family,
struct sk_buff *skb, struct net *net, u32 portid,
u32 group, struct nlmsghdr *nlh, gfp_t flags);
+struct sk_buff *genlmsg_new_unicast(size_t payload, struct genl_info *info,
+ gfp_t flags);
void *genlmsg_put(struct sk_buff *skb, u32 portid, u32 seq,
struct genl_family *family, int flags, u8 cmd);
diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
index 7dbc4f7..51bdc12 100644
--- a/net/netlink/genetlink.c
+++ b/net/netlink/genetlink.c
@@ -454,6 +454,26 @@ int genl_unregister_family(struct genl_family *family)
EXPORT_SYMBOL(genl_unregister_family);
/**
+ * genlmsg_new_unicast - Allocate generic netlink message for unicast
+ * @payload: size of the message payload
+ * @info: information on destination
+ * @flags: the type of memory to allocate
+ *
+ * Allocates a new sk_buff large enough to cover the specified payload
+ * plus required Netlink headers. Will check receiving socket for
+ * memory mapped i/o capability and use it if enabled. Will fall back
+ * to non-mapped skb if message size exceeds the frame size of the ring.
+ */
+struct sk_buff *genlmsg_new_unicast(size_t payload, struct genl_info *info,
+ gfp_t flags)
+{
+ size_t len = nlmsg_total_size(genlmsg_total_size(payload));
+
+ return netlink_alloc_skb(info->dst_sk, len, info->snd_portid, flags);
+}
+EXPORT_SYMBOL_GPL(genlmsg_new_unicast);
+
+/**
* genlmsg_put - Add generic netlink header to netlink message
* @skb: socket buffer holding the message
* @portid: netlink portid the message is addressed to
@@ -593,6 +613,7 @@ static int genl_family_rcv_msg(struct genl_family *family,
info.genlhdr = nlmsg_data(nlh);
info.userhdr = nlmsg_data(nlh) + GENL_HDRLEN;
info.attrs = attrbuf;
+ info.dst_sk = skb->sk;
genl_info_net_set(&info, net);
memset(&info.user_ptr, 0, sizeof(info.user_ptr));
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next 0/8 v6] Open vSwitch upcall optimiziations
From: Thomas Graf @ 2013-11-21 18:13 UTC (permalink / raw)
To: jesse, davem
Cc: dev, netdev, dborkman, ffusco, fleitner, eric.dumazet, bhutchings
Reposting this before the merge window as it will go through Jesse's
tree.
Given jumbo frames, the capacity of the slow path is improved by
a factor of 2.5x.
V6: - Added memory mapped netlink i/o support
- Drop user_features if old user space not aware of user features
reuses an existing datapath
V5: - Removed padding requirement in user space
- Added OVS_DP_F_UNALIGNED flag let user space signal ability to
receive unaligned Netlink messages, fall back to linear copy
otherwise.
V4: - Daniel Borkmann pointed out that the style in skbuff.h has changed
in net-next, adapted to no longer using extern in headers.
V3: - Removed unneeded alignment of nlmsg_len after padding
V2: - Added skb_zerocopy_headlen() to calculate headroom of destination
buffer. This also takes care of the from->head_frag issue.
- Attribute alignment for frag_list case
- API documentation
- performance data for CHECKSUM_PARTIAL tx case
Thomas Graf (8):
genl: Add genlmsg_new_unicast() for unicast message allocation
netlink: Avoid netlink mmap alloc if msg size exceeds frame size
openvswitch: Enable memory mapped Netlink i/o
net: Export skb_zerocopy() to zerocopy from one skb to another
openvswitch: Allow user space to announce ability to accept unaligned
Netlink messages
openvswitch: Allow update of dp with OVS_DP_CMD_NEW if NLM_F_REPLACE
is set
openvswitch: Drop user features if old user space attempted to create
datapath
openvswitch: Use skb_zerocopy() for upcall
include/linux/skbuff.h | 3 +
include/net/genetlink.h | 4 +
include/uapi/linux/openvswitch.h | 15 ++-
net/core/skbuff.c | 85 +++++++++++++++
net/netfilter/nfnetlink_queue_core.c | 59 +----------
net/netlink/af_netlink.c | 4 +
net/netlink/genetlink.c | 21 ++++
net/openvswitch/datapath.c | 193 ++++++++++++++++++++++-------------
net/openvswitch/datapath.h | 2 +
9 files changed, 257 insertions(+), 129 deletions(-)
--
1.8.3.1
^ permalink raw reply
* [PATCH net-next 2/8] netlink: Avoid netlink mmap alloc if msg size exceeds frame size
From: Thomas Graf @ 2013-11-21 18:13 UTC (permalink / raw)
To: jesse, davem
Cc: dev, netdev, dborkman, ffusco, fleitner, eric.dumazet, bhutchings
In-Reply-To: <cover.1385057355.git.tgraf@suug.ch>
An insufficent ring frame size configuration can lead to an
unnecessary skb allocation for every Netlink message. Check frame
size before taking the queue lock and allocating the skb and
re-check with lock to be safe.
Signed-off-by: Thomas Graf <tgraf@suug.ch>
Reviewed-by: Daniel Borkmann <dborkman@redhat.com>
---
net/netlink/af_netlink.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index bca50b9..6433489 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -1769,6 +1769,9 @@ struct sk_buff *netlink_alloc_skb(struct sock *ssk, unsigned int size,
if (ring->pg_vec == NULL)
goto out_put;
+ if (ring->frame_size - NL_MMAP_HDRLEN < size)
+ goto out_put;
+
skb = alloc_skb_head(gfp_mask);
if (skb == NULL)
goto err1;
@@ -1778,6 +1781,7 @@ struct sk_buff *netlink_alloc_skb(struct sock *ssk, unsigned int size,
if (ring->pg_vec == NULL)
goto out_free;
+ /* check again under lock */
maxlen = ring->frame_size - NL_MMAP_HDRLEN;
if (maxlen < size)
goto out_free;
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next 5/8] openvswitch: Allow user space to announce ability to accept unaligned Netlink messages
From: Thomas Graf @ 2013-11-21 18:13 UTC (permalink / raw)
To: jesse, davem
Cc: dev, netdev, dborkman, ffusco, fleitner, eric.dumazet, bhutchings
In-Reply-To: <cover.1385057355.git.tgraf@suug.ch>
Signed-off-by: Thomas Graf <tgraf@suug.ch>
Reviewed-by: Daniel Borkmann <dborkman@redhat.com>
---
include/uapi/linux/openvswitch.h | 4 ++++
net/openvswitch/datapath.c | 14 ++++++++++++++
net/openvswitch/datapath.h | 2 ++
3 files changed, 20 insertions(+)
diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
index d120f9f..07ef2c3 100644
--- a/include/uapi/linux/openvswitch.h
+++ b/include/uapi/linux/openvswitch.h
@@ -75,6 +75,7 @@ enum ovs_datapath_attr {
OVS_DP_ATTR_UPCALL_PID, /* Netlink PID to receive upcalls */
OVS_DP_ATTR_STATS, /* struct ovs_dp_stats */
OVS_DP_ATTR_MEGAFLOW_STATS, /* struct ovs_dp_megaflow_stats */
+ OVS_DP_ATTR_USER_FEATURES, /* OVS_DP_F_* */
__OVS_DP_ATTR_MAX
};
@@ -106,6 +107,9 @@ struct ovs_vport_stats {
__u64 tx_dropped; /* no space available in linux */
};
+/* Allow last Netlink attribute to be unaligned */
+#define OVS_DP_F_UNALIGNED (1 << 0)
+
/* Fixed logical ports. */
#define OVSP_LOCAL ((__u32)0)
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index 0ac9cde..95d4424 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -1067,6 +1067,7 @@ static const struct genl_ops dp_flow_genl_ops[] = {
static const struct nla_policy datapath_policy[OVS_DP_ATTR_MAX + 1] = {
[OVS_DP_ATTR_NAME] = { .type = NLA_NUL_STRING, .len = IFNAMSIZ - 1 },
[OVS_DP_ATTR_UPCALL_PID] = { .type = NLA_U32 },
+ [OVS_DP_ATTR_USER_FEATURES] = { .type = NLA_U32 },
};
static struct genl_family dp_datapath_genl_family = {
@@ -1125,6 +1126,9 @@ static int ovs_dp_cmd_fill_info(struct datapath *dp, struct sk_buff *skb,
&dp_megaflow_stats))
goto nla_put_failure;
+ if (nla_put_u32(skb, OVS_DP_ATTR_USER_FEATURES, dp->user_features))
+ goto nla_put_failure;
+
return genlmsg_end(skb, ovs_header);
nla_put_failure:
@@ -1171,6 +1175,12 @@ static struct datapath *lookup_datapath(struct net *net,
return dp ? dp : ERR_PTR(-ENODEV);
}
+static void ovs_dp_change(struct datapath *dp, struct nlattr **a)
+{
+ if (a[OVS_DP_ATTR_USER_FEATURES])
+ dp->user_features = nla_get_u32(a[OVS_DP_ATTR_USER_FEATURES]);
+}
+
static int ovs_dp_cmd_new(struct sk_buff *skb, struct genl_info *info)
{
struct nlattr **a = info->attrs;
@@ -1229,6 +1239,8 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct genl_info *info)
parms.port_no = OVSP_LOCAL;
parms.upcall_portid = nla_get_u32(a[OVS_DP_ATTR_UPCALL_PID]);
+ ovs_dp_change(dp, a);
+
vport = new_vport(&parms);
if (IS_ERR(vport)) {
err = PTR_ERR(vport);
@@ -1332,6 +1344,8 @@ static int ovs_dp_cmd_set(struct sk_buff *skb, struct genl_info *info)
if (IS_ERR(dp))
goto unlock;
+ ovs_dp_change(dp, info->attrs);
+
reply = ovs_dp_cmd_build_info(dp, info, OVS_DP_CMD_NEW);
if (IS_ERR(reply)) {
err = PTR_ERR(reply);
diff --git a/net/openvswitch/datapath.h b/net/openvswitch/datapath.h
index 4067ea4..193e2e0 100644
--- a/net/openvswitch/datapath.h
+++ b/net/openvswitch/datapath.h
@@ -88,6 +88,8 @@ struct datapath {
/* Network namespace ref. */
struct net *net;
#endif
+
+ u32 user_features;
};
/**
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next 3/8] openvswitch: Enable memory mapped Netlink i/o
From: Thomas Graf @ 2013-11-21 18:13 UTC (permalink / raw)
To: jesse, davem
Cc: dev, netdev, dborkman, ffusco, fleitner, eric.dumazet, bhutchings
In-Reply-To: <cover.1385057355.git.tgraf@suug.ch>
Use memory mapped Netlink i/o for all unicast openvswitch
communication if a ring has been set up.
Benchmark
* pktgen -> ovs internal port
* 5M pkts, 5M flows
* 4 threads, 8 cores
Before:
Result: OK: 67418743(c67108212+d310530) usec, 5000000 (9000byte,0frags)
74163pps 5339Mb/sec (5339736000bps) errors: 0
+ 2.98% ovs-vswitchd [k] copy_user_generic_string
+ 2.49% ovs-vswitchd [k] memcpy
+ 1.84% kpktgend_2 [k] memcpy
+ 1.81% kpktgend_1 [k] memcpy
+ 1.81% kpktgend_3 [k] memcpy
+ 1.78% kpktgend_0 [k] memcpy
After:
Result: OK: 24229690(c24127165+d102524) usec, 5000000 (9000byte,0frags)
206358pps 14857Mb/sec (14857776000bps) errors: 0
+ 2.80% ovs-vswitchd [k] memcpy
+ 1.31% kpktgend_2 [k] memcpy
+ 1.23% kpktgend_0 [k] memcpy
+ 1.09% kpktgend_1 [k] memcpy
+ 1.04% kpktgend_3 [k] memcpy
+ 0.96% ovs-vswitchd [k] copy_user_generic_string
Signed-off-by: Thomas Graf <tgraf@suug.ch>
Reviewed-by: Daniel Borkmann <dborkman@redhat.com>
---
net/openvswitch/datapath.c | 56 ++++++++++++++++++++++++----------------------
1 file changed, 29 insertions(+), 27 deletions(-)
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index 6f5e1dd..0ac9cde 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -402,6 +402,11 @@ static int queue_userspace_packet(struct net *net, int dp_ifindex,
struct sk_buff *nskb = NULL;
struct sk_buff *user_skb; /* to be queued to userspace */
struct nlattr *nla;
+ struct genl_info info = {
+ .dst_sk = net->genl_sock,
+ .snd_portid = upcall_info->portid,
+ };
+ size_t len;
int err;
if (vlan_tx_tag_present(skb)) {
@@ -422,7 +427,8 @@ static int queue_userspace_packet(struct net *net, int dp_ifindex,
goto out;
}
- user_skb = genlmsg_new(upcall_msg_size(skb, upcall_info->userdata), GFP_ATOMIC);
+ len = upcall_msg_size(skb, upcall_info->userdata);
+ user_skb = genlmsg_new_unicast(len, &info, GFP_ATOMIC);
if (!user_skb) {
err = -ENOMEM;
goto out;
@@ -726,27 +732,30 @@ error:
return err;
}
-static struct sk_buff *ovs_flow_cmd_alloc_info(struct sw_flow *flow)
+static struct sk_buff *ovs_flow_cmd_alloc_info(struct sw_flow *flow,
+ struct genl_info *info)
{
- const struct sw_flow_actions *sf_acts;
+ size_t len;
- sf_acts = ovsl_dereference(flow->sf_acts);
+ len = ovs_flow_cmd_msg_size(ovsl_dereference(flow->sf_acts));
- return genlmsg_new(ovs_flow_cmd_msg_size(sf_acts), GFP_KERNEL);
+ return genlmsg_new_unicast(len, info, GFP_KERNEL);
}
static struct sk_buff *ovs_flow_cmd_build_info(struct sw_flow *flow,
struct datapath *dp,
- u32 portid, u32 seq, u8 cmd)
+ struct genl_info *info,
+ u8 cmd)
{
struct sk_buff *skb;
int retval;
- skb = ovs_flow_cmd_alloc_info(flow);
+ skb = ovs_flow_cmd_alloc_info(flow, info);
if (!skb)
return ERR_PTR(-ENOMEM);
- retval = ovs_flow_cmd_fill_info(flow, dp, skb, portid, seq, 0, cmd);
+ retval = ovs_flow_cmd_fill_info(flow, dp, skb, info->snd_portid,
+ info->snd_seq, 0, cmd);
BUG_ON(retval < 0);
return skb;
}
@@ -835,8 +844,7 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb, struct genl_info *info)
goto err_flow_free;
}
- reply = ovs_flow_cmd_build_info(flow, dp, info->snd_portid,
- info->snd_seq, OVS_FLOW_CMD_NEW);
+ reply = ovs_flow_cmd_build_info(flow, dp, info, OVS_FLOW_CMD_NEW);
} else {
/* We found a matching flow. */
struct sw_flow_actions *old_acts;
@@ -864,8 +872,7 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb, struct genl_info *info)
rcu_assign_pointer(flow->sf_acts, acts);
ovs_nla_free_flow_actions(old_acts);
- reply = ovs_flow_cmd_build_info(flow, dp, info->snd_portid,
- info->snd_seq, OVS_FLOW_CMD_NEW);
+ reply = ovs_flow_cmd_build_info(flow, dp, info, OVS_FLOW_CMD_NEW);
/* Clear stats. */
if (a[OVS_FLOW_ATTR_CLEAR]) {
@@ -927,8 +934,7 @@ static int ovs_flow_cmd_get(struct sk_buff *skb, struct genl_info *info)
goto unlock;
}
- reply = ovs_flow_cmd_build_info(flow, dp, info->snd_portid,
- info->snd_seq, OVS_FLOW_CMD_NEW);
+ reply = ovs_flow_cmd_build_info(flow, dp, info, OVS_FLOW_CMD_NEW);
if (IS_ERR(reply)) {
err = PTR_ERR(reply);
goto unlock;
@@ -975,7 +981,7 @@ static int ovs_flow_cmd_del(struct sk_buff *skb, struct genl_info *info)
goto unlock;
}
- reply = ovs_flow_cmd_alloc_info(flow);
+ reply = ovs_flow_cmd_alloc_info(flow, info);
if (!reply) {
err = -ENOMEM;
goto unlock;
@@ -1127,17 +1133,17 @@ error:
return -EMSGSIZE;
}
-static struct sk_buff *ovs_dp_cmd_build_info(struct datapath *dp, u32 portid,
- u32 seq, u8 cmd)
+static struct sk_buff *ovs_dp_cmd_build_info(struct datapath *dp,
+ struct genl_info *info, u8 cmd)
{
struct sk_buff *skb;
int retval;
- skb = genlmsg_new(ovs_dp_cmd_msg_size(), GFP_KERNEL);
+ skb = genlmsg_new_unicast(ovs_dp_cmd_msg_size(), info, GFP_KERNEL);
if (!skb)
return ERR_PTR(-ENOMEM);
- retval = ovs_dp_cmd_fill_info(dp, skb, portid, seq, 0, cmd);
+ retval = ovs_dp_cmd_fill_info(dp, skb, info->snd_portid, info->snd_seq, 0, cmd);
if (retval < 0) {
kfree_skb(skb);
return ERR_PTR(retval);
@@ -1232,8 +1238,7 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct genl_info *info)
goto err_destroy_ports_array;
}
- reply = ovs_dp_cmd_build_info(dp, info->snd_portid,
- info->snd_seq, OVS_DP_CMD_NEW);
+ reply = ovs_dp_cmd_build_info(dp, info, OVS_DP_CMD_NEW);
err = PTR_ERR(reply);
if (IS_ERR(reply))
goto err_destroy_local_port;
@@ -1299,8 +1304,7 @@ static int ovs_dp_cmd_del(struct sk_buff *skb, struct genl_info *info)
if (IS_ERR(dp))
goto unlock;
- reply = ovs_dp_cmd_build_info(dp, info->snd_portid,
- info->snd_seq, OVS_DP_CMD_DEL);
+ reply = ovs_dp_cmd_build_info(dp, info, OVS_DP_CMD_DEL);
err = PTR_ERR(reply);
if (IS_ERR(reply))
goto unlock;
@@ -1328,8 +1332,7 @@ static int ovs_dp_cmd_set(struct sk_buff *skb, struct genl_info *info)
if (IS_ERR(dp))
goto unlock;
- reply = ovs_dp_cmd_build_info(dp, info->snd_portid,
- info->snd_seq, OVS_DP_CMD_NEW);
+ reply = ovs_dp_cmd_build_info(dp, info, OVS_DP_CMD_NEW);
if (IS_ERR(reply)) {
err = PTR_ERR(reply);
genl_set_err(&dp_datapath_genl_family, sock_net(skb->sk), 0,
@@ -1360,8 +1363,7 @@ static int ovs_dp_cmd_get(struct sk_buff *skb, struct genl_info *info)
goto unlock;
}
- reply = ovs_dp_cmd_build_info(dp, info->snd_portid,
- info->snd_seq, OVS_DP_CMD_NEW);
+ reply = ovs_dp_cmd_build_info(dp, info, OVS_DP_CMD_NEW);
if (IS_ERR(reply)) {
err = PTR_ERR(reply);
goto unlock;
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next 4/8] net: Export skb_zerocopy() to zerocopy from one skb to another
From: Thomas Graf @ 2013-11-21 18:13 UTC (permalink / raw)
To: jesse, davem
Cc: dev, netdev, dborkman, ffusco, fleitner, eric.dumazet, bhutchings
In-Reply-To: <cover.1385057355.git.tgraf@suug.ch>
Make the skb zerocopy logic written for nfnetlink queue available for
use by other modules.
Signed-off-by: Thomas Graf <tgraf@suug.ch>
Reviewed-by: Daniel Borkmann <dborkman@redhat.com>
---
include/linux/skbuff.h | 3 ++
net/core/skbuff.c | 85 ++++++++++++++++++++++++++++++++++++
net/netfilter/nfnetlink_queue_core.c | 59 ++-----------------------
3 files changed, 92 insertions(+), 55 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index bec1cc7..7c48e2d 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2345,6 +2345,9 @@ int skb_splice_bits(struct sk_buff *skb, unsigned int offset,
struct pipe_inode_info *pipe, unsigned int len,
unsigned int flags);
void skb_copy_and_csum_dev(const struct sk_buff *skb, u8 *to);
+unsigned int skb_zerocopy_headlen(const struct sk_buff *from);
+void skb_zerocopy(struct sk_buff *to, const struct sk_buff *from,
+ int len, int hlen);
void skb_split(struct sk_buff *skb, struct sk_buff *skb1, const u32 len);
int skb_shift(struct sk_buff *tgt, struct sk_buff *skb, int shiftlen);
void skb_scrub_packet(struct sk_buff *skb, bool xnet);
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 8cec1e6..b448fc0 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -2122,6 +2122,91 @@ __wsum skb_copy_and_csum_bits(const struct sk_buff *skb, int offset,
}
EXPORT_SYMBOL(skb_copy_and_csum_bits);
+ /**
+ * skb_zerocopy_headlen - Calculate headroom needed for skb_zerocopy()
+ * @from: source buffer
+ *
+ * Calculates the amount of linear headroom needed in the 'to' skb passed
+ * into skb_zerocopy().
+ */
+unsigned int
+skb_zerocopy_headlen(const struct sk_buff *from)
+{
+ unsigned int hlen = 0;
+
+ if (!from->head_frag ||
+ skb_headlen(from) < L1_CACHE_BYTES ||
+ skb_shinfo(from)->nr_frags >= MAX_SKB_FRAGS)
+ hlen = skb_headlen(from);
+
+ if (skb_has_frag_list(from))
+ hlen = from->len;
+
+ return hlen;
+}
+EXPORT_SYMBOL_GPL(skb_zerocopy_headlen);
+
+/**
+ * skb_zerocopy - Zero copy skb to skb
+ * @to: destination buffer
+ * @source: source buffer
+ * @len: number of bytes to copy from source buffer
+ * @hlen: size of linear headroom in destination buffer
+ *
+ * Copies up to `len` bytes from `from` to `to` by creating references
+ * to the frags in the source buffer.
+ *
+ * The `hlen` as calculated by skb_zerocopy_headlen() specifies the
+ * headroom in the `to` buffer.
+ */
+void
+skb_zerocopy(struct sk_buff *to, const struct sk_buff *from, int len, int hlen)
+{
+ int i, j = 0;
+ int plen = 0; /* length of skb->head fragment */
+ struct page *page;
+ unsigned int offset;
+
+ BUG_ON(!from->head_frag && !hlen);
+
+ /* dont bother with small payloads */
+ if (len <= skb_tailroom(to)) {
+ skb_copy_bits(from, 0, skb_put(to, len), len);
+ return;
+ }
+
+ if (hlen) {
+ skb_copy_bits(from, 0, skb_put(to, hlen), hlen);
+ len -= hlen;
+ } else {
+ plen = min_t(int, skb_headlen(from), len);
+ if (plen) {
+ page = virt_to_head_page(from->head);
+ offset = from->data - (unsigned char *)page_address(page);
+ __skb_fill_page_desc(to, 0, page, offset, plen);
+ get_page(page);
+ j = 1;
+ len -= plen;
+ }
+ }
+
+ to->truesize += len + plen;
+ to->len += len + plen;
+ to->data_len += len + plen;
+
+ for (i = 0; i < skb_shinfo(from)->nr_frags; i++) {
+ if (!len)
+ break;
+ skb_shinfo(to)->frags[j] = skb_shinfo(from)->frags[i];
+ skb_shinfo(to)->frags[j].size = min_t(int, skb_shinfo(to)->frags[j].size, len);
+ len -= skb_shinfo(to)->frags[j].size;
+ skb_frag_ref(to, j);
+ j++;
+ }
+ skb_shinfo(to)->nr_frags = j;
+}
+EXPORT_SYMBOL_GPL(skb_zerocopy);
+
void skb_copy_and_csum_dev(const struct sk_buff *skb, u8 *to)
{
__wsum csum;
diff --git a/net/netfilter/nfnetlink_queue_core.c b/net/netfilter/nfnetlink_queue_core.c
index 21258cf..615ee12 100644
--- a/net/netfilter/nfnetlink_queue_core.c
+++ b/net/netfilter/nfnetlink_queue_core.c
@@ -235,51 +235,6 @@ nfqnl_flush(struct nfqnl_instance *queue, nfqnl_cmpfn cmpfn, unsigned long data)
spin_unlock_bh(&queue->lock);
}
-static void
-nfqnl_zcopy(struct sk_buff *to, const struct sk_buff *from, int len, int hlen)
-{
- int i, j = 0;
- int plen = 0; /* length of skb->head fragment */
- struct page *page;
- unsigned int offset;
-
- /* dont bother with small payloads */
- if (len <= skb_tailroom(to)) {
- skb_copy_bits(from, 0, skb_put(to, len), len);
- return;
- }
-
- if (hlen) {
- skb_copy_bits(from, 0, skb_put(to, hlen), hlen);
- len -= hlen;
- } else {
- plen = min_t(int, skb_headlen(from), len);
- if (plen) {
- page = virt_to_head_page(from->head);
- offset = from->data - (unsigned char *)page_address(page);
- __skb_fill_page_desc(to, 0, page, offset, plen);
- get_page(page);
- j = 1;
- len -= plen;
- }
- }
-
- to->truesize += len + plen;
- to->len += len + plen;
- to->data_len += len + plen;
-
- for (i = 0; i < skb_shinfo(from)->nr_frags; i++) {
- if (!len)
- break;
- skb_shinfo(to)->frags[j] = skb_shinfo(from)->frags[i];
- skb_shinfo(to)->frags[j].size = min_t(int, skb_shinfo(to)->frags[j].size, len);
- len -= skb_shinfo(to)->frags[j].size;
- skb_frag_ref(to, j);
- j++;
- }
- skb_shinfo(to)->nr_frags = j;
-}
-
static int
nfqnl_put_packet_info(struct sk_buff *nlskb, struct sk_buff *packet,
bool csum_verify)
@@ -304,7 +259,7 @@ nfqnl_build_packet_message(struct net *net, struct nfqnl_instance *queue,
{
size_t size;
size_t data_len = 0, cap_len = 0;
- int hlen = 0;
+ unsigned int hlen = 0;
struct sk_buff *skb;
struct nlattr *nla;
struct nfqnl_msg_packet_hdr *pmsg;
@@ -356,14 +311,8 @@ nfqnl_build_packet_message(struct net *net, struct nfqnl_instance *queue,
if (data_len > entskb->len)
data_len = entskb->len;
- if (!entskb->head_frag ||
- skb_headlen(entskb) < L1_CACHE_BYTES ||
- skb_shinfo(entskb)->nr_frags >= MAX_SKB_FRAGS)
- hlen = skb_headlen(entskb);
-
- if (skb_has_frag_list(entskb))
- hlen = entskb->len;
- hlen = min_t(int, data_len, hlen);
+ hlen = skb_zerocopy_headlen(entskb);
+ hlen = min_t(unsigned int, hlen, data_len);
size += sizeof(struct nlattr) + hlen;
cap_len = entskb->len;
break;
@@ -504,7 +453,7 @@ nfqnl_build_packet_message(struct net *net, struct nfqnl_instance *queue,
nla->nla_type = NFQA_PAYLOAD;
nla->nla_len = nla_attr_size(data_len);
- nfqnl_zcopy(skb, entskb, data_len, hlen);
+ skb_zerocopy(skb, entskb, data_len, hlen);
}
nlh->nlmsg_len = skb->len;
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next 6/8] openvswitch: Allow update of dp with OVS_DP_CMD_NEW if NLM_F_REPLACE is set
From: Thomas Graf @ 2013-11-21 18:13 UTC (permalink / raw)
To: jesse, davem
Cc: dev, netdev, dborkman, ffusco, fleitner, eric.dumazet, bhutchings
In-Reply-To: <cover.1385057355.git.tgraf@suug.ch>
Consolidates ovs_dp_cmd_new() and ovs_dp_cmd_set() to simplify
handling and avoid code duplication.
Allows user space to specify NLM_F_REPLACE with OVS_DP_CMD_NEW and
overwrite the settings such as the user features of an existing
datapath.
Signed-off-by: Thomas Graf <tgraf@suug.ch>
Reviewed-by: Daniel Borkmann <dborkman@redhat.com>
---
net/openvswitch/datapath.c | 83 +++++++++++++++++++++++-----------------------
1 file changed, 42 insertions(+), 41 deletions(-)
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index 95d4424..3f1fb87 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -1175,13 +1175,8 @@ static struct datapath *lookup_datapath(struct net *net,
return dp ? dp : ERR_PTR(-ENODEV);
}
-static void ovs_dp_change(struct datapath *dp, struct nlattr **a)
-{
- if (a[OVS_DP_ATTR_USER_FEATURES])
- dp->user_features = nla_get_u32(a[OVS_DP_ATTR_USER_FEATURES]);
-}
-
-static int ovs_dp_cmd_new(struct sk_buff *skb, struct genl_info *info)
+static int ovs_dp_create_or_update(struct sk_buff *skb, struct genl_info *info,
+ bool create)
{
struct nlattr **a = info->attrs;
struct vport_parms parms;
@@ -1190,6 +1185,7 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct genl_info *info)
struct vport *vport;
struct ovs_net *ovs_net;
int err, i;
+ bool allocated = false;
err = -EINVAL;
if (!a[OVS_DP_ATTR_NAME] || !a[OVS_DP_ATTR_UPCALL_PID])
@@ -1197,11 +1193,26 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct genl_info *info)
ovs_lock();
+ dp = lookup_datapath(sock_net(skb->sk), info->userhdr, info->attrs);
+ if (!IS_ERR(dp)) {
+ if (info->nlhdr->nlmsg_flags & NLM_F_REPLACE)
+ goto update;
+
+ err = -EEXIST;
+ goto err_unlock_ovs;
+ }
+
+ if (!create) {
+ err = PTR_ERR(dp);
+ goto err_unlock_ovs;
+ }
+
err = -ENOMEM;
dp = kzalloc(sizeof(*dp), GFP_KERNEL);
if (dp == NULL)
goto err_unlock_ovs;
+ allocated = true;
ovs_dp_set_net(dp, hold_net(sock_net(skb->sk)));
/* Allocate table. */
@@ -1239,8 +1250,6 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct genl_info *info)
parms.port_no = OVSP_LOCAL;
parms.upcall_portid = nla_get_u32(a[OVS_DP_ATTR_UPCALL_PID]);
- ovs_dp_change(dp, a);
-
vport = new_vport(&parms);
if (IS_ERR(vport)) {
err = PTR_ERR(vport);
@@ -1250,13 +1259,27 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct genl_info *info)
goto err_destroy_ports_array;
}
+update:
+ if (a[OVS_DP_ATTR_USER_FEATURES])
+ dp->user_features = nla_get_u32(a[OVS_DP_ATTR_USER_FEATURES]);
+
reply = ovs_dp_cmd_build_info(dp, info, OVS_DP_CMD_NEW);
err = PTR_ERR(reply);
- if (IS_ERR(reply))
- goto err_destroy_local_port;
+ if (IS_ERR(reply)) {
+ if (allocated)
+ goto err_destroy_local_port;
+ else {
+ err = 0;
+ genl_set_err(&dp_datapath_genl_family, sock_net(skb->sk), 0,
+ 0, err);
+ goto err_unlock_ovs;
+ }
+ }
- ovs_net = net_generic(ovs_dp_get_net(dp), ovs_net_id);
- list_add_tail_rcu(&dp->list_node, &ovs_net->dps);
+ if (allocated) {
+ ovs_net = net_generic(ovs_dp_get_net(dp), ovs_net_id);
+ list_add_tail_rcu(&dp->list_node, &ovs_net->dps);
+ }
ovs_unlock();
@@ -1280,6 +1303,11 @@ err:
return err;
}
+static int ovs_dp_cmd_new(struct sk_buff *skb, struct genl_info *info)
+{
+ return ovs_dp_create_or_update(skb, info, true);
+}
+
/* Called with ovs_mutex. */
static void __dp_destroy(struct datapath *dp)
{
@@ -1334,34 +1362,7 @@ unlock:
static int ovs_dp_cmd_set(struct sk_buff *skb, struct genl_info *info)
{
- struct sk_buff *reply;
- struct datapath *dp;
- int err;
-
- ovs_lock();
- dp = lookup_datapath(sock_net(skb->sk), info->userhdr, info->attrs);
- err = PTR_ERR(dp);
- if (IS_ERR(dp))
- goto unlock;
-
- ovs_dp_change(dp, info->attrs);
-
- reply = ovs_dp_cmd_build_info(dp, info, OVS_DP_CMD_NEW);
- if (IS_ERR(reply)) {
- err = PTR_ERR(reply);
- genl_set_err(&dp_datapath_genl_family, sock_net(skb->sk), 0,
- 0, err);
- err = 0;
- goto unlock;
- }
-
- ovs_unlock();
- ovs_notify(&dp_datapath_genl_family, reply, info);
-
- return 0;
-unlock:
- ovs_unlock();
- return err;
+ return ovs_dp_create_or_update(skb, info, false);
}
static int ovs_dp_cmd_get(struct sk_buff *skb, struct genl_info *info)
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next 7/8] openvswitch: Drop user features if old user space attempted to create datapath
From: Thomas Graf @ 2013-11-21 18:13 UTC (permalink / raw)
To: jesse, davem
Cc: dev, netdev, dborkman, ffusco, fleitner, eric.dumazet, bhutchings
In-Reply-To: <cover.1385057355.git.tgraf@suug.ch>
Drop user features if an outdated user space instance that does not
understand the concept of user_features attempted to create a new
datapath.
Signed-off-by: Thomas Graf <tgraf@suug.ch>
Reviewed-by: Daniel Borkmann <dborkman@redhat.com>
---
include/uapi/linux/openvswitch.h | 11 ++++++++++-
net/openvswitch/datapath.c | 10 ++++++++++
2 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
index 07ef2c3..4f74c2c 100644
--- a/include/uapi/linux/openvswitch.h
+++ b/include/uapi/linux/openvswitch.h
@@ -40,7 +40,16 @@ struct ovs_header {
#define OVS_DATAPATH_FAMILY "ovs_datapath"
#define OVS_DATAPATH_MCGROUP "ovs_datapath"
-#define OVS_DATAPATH_VERSION 0x1
+
+/**
+ * V2:
+ * - API users are expected to provide OVS_DP_ATTR_USER_FEATURES
+ * with NLM_F_REPLACE when creating the datapath.
+ */
+#define OVS_DATAPATH_VERSION 2
+
+/* First OVS datapath version to support features */
+#define OVS_DP_VER_FEATURES 2
enum ovs_datapath_cmd {
OVS_DP_CMD_UNSPEC,
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index 3f1fb87..72cdffb 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -1198,6 +1198,16 @@ static int ovs_dp_create_or_update(struct sk_buff *skb, struct genl_info *info,
if (info->nlhdr->nlmsg_flags & NLM_F_REPLACE)
goto update;
+ /* An outdated user space instance that does not understand
+ * the concept of user_features has attempted to create a new
+ * datapath. Drop all user features.
+ */
+ if (info->genlhdr->version < OVS_DP_VER_FEATURES) {
+ WARN_ONCE(dp->user_features, "Dropping previously "
+ "announced user features");
+ dp->user_features = 0;
+ }
+
err = -EEXIST;
goto err_unlock_ovs;
}
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next 8/8] openvswitch: Use skb_zerocopy() for upcall
From: Thomas Graf @ 2013-11-21 18:13 UTC (permalink / raw)
To: jesse, davem
Cc: dev, netdev, dborkman, ffusco, fleitner, eric.dumazet, bhutchings
In-Reply-To: <cover.1385057355.git.tgraf@suug.ch>
Use of skb_zerocopy() can avoids the expensive call to memcpy()
when copying the packet data into the Netlink skb. Completes
checksum through skb_checksum_help() if needed.
Zerocopy is only performed if user space supported unaligned
Netlink messages. memory mapped netlink i/o is preferred over
zerocopy if it is set up.
Cost of upcall is significantly reduced from:
+ 7.48% vhost-8471 [k] memcpy
+ 5.57% ovs-vswitchd [k] memcpy
+ 2.81% vhost-8471 [k] csum_partial_copy_generic
to:
+ 5.72% ovs-vswitchd [k] memcpy
+ 3.32% vhost-5153 [k] memcpy
+ 0.68% vhost-5153 [k] skb_zerocopy
(megaflows disabled)
Signed-off-by: Thomas Graf <tgraf@suug.ch>
Reviewed-by: Daniel Borkmann <dborkman@redhat.com>
---
net/openvswitch/datapath.c | 54 +++++++++++++++++++++++++++++++---------------
1 file changed, 37 insertions(+), 17 deletions(-)
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index 72cdffb..237f7d7 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -108,10 +108,10 @@ int lockdep_ovsl_is_held(void)
#endif
static struct vport *new_vport(const struct vport_parms *);
-static int queue_gso_packets(struct net *, int dp_ifindex, struct sk_buff *,
- const struct dp_upcall_info *);
-static int queue_userspace_packet(struct net *, int dp_ifindex,
- struct sk_buff *,
+static int queue_gso_packets(struct datapath *, struct net *, int dp_ifindex,
+ struct sk_buff *, const struct dp_upcall_info *);
+static int queue_userspace_packet(struct datapath *, struct net *,
+ int dp_ifindex, struct sk_buff *,
const struct dp_upcall_info *);
/* Must be called with rcu_read_lock or ovs_mutex. */
@@ -292,9 +292,9 @@ int ovs_dp_upcall(struct datapath *dp, struct sk_buff *skb,
}
if (!skb_is_gso(skb))
- err = queue_userspace_packet(ovs_dp_get_net(dp), dp_ifindex, skb, upcall_info);
+ err = queue_userspace_packet(dp, ovs_dp_get_net(dp), dp_ifindex, skb, upcall_info);
else
- err = queue_gso_packets(ovs_dp_get_net(dp), dp_ifindex, skb, upcall_info);
+ err = queue_gso_packets(dp, ovs_dp_get_net(dp), dp_ifindex, skb, upcall_info);
if (err)
goto err;
@@ -310,7 +310,7 @@ err:
return err;
}
-static int queue_gso_packets(struct net *net, int dp_ifindex,
+static int queue_gso_packets(struct datapath *dp, struct net *net, int dp_ifindex,
struct sk_buff *skb,
const struct dp_upcall_info *upcall_info)
{
@@ -327,7 +327,7 @@ static int queue_gso_packets(struct net *net, int dp_ifindex,
/* Queue all of the segments. */
skb = segs;
do {
- err = queue_userspace_packet(net, dp_ifindex, skb, upcall_info);
+ err = queue_userspace_packet(dp, net, dp_ifindex, skb, upcall_info);
if (err)
break;
@@ -381,10 +381,11 @@ static size_t key_attr_size(void)
}
static size_t upcall_msg_size(const struct sk_buff *skb,
- const struct nlattr *userdata)
+ const struct nlattr *userdata,
+ unsigned int hdrlen)
{
size_t size = NLMSG_ALIGN(sizeof(struct ovs_header))
- + nla_total_size(skb->len) /* OVS_PACKET_ATTR_PACKET */
+ + nla_total_size(hdrlen) /* OVS_PACKET_ATTR_PACKET */
+ nla_total_size(key_attr_size()); /* OVS_PACKET_ATTR_KEY */
/* OVS_PACKET_ATTR_USERDATA */
@@ -394,8 +395,8 @@ static size_t upcall_msg_size(const struct sk_buff *skb,
return size;
}
-static int queue_userspace_packet(struct net *net, int dp_ifindex,
- struct sk_buff *skb,
+static int queue_userspace_packet(struct datapath *dp, struct net *net,
+ int dp_ifindex, struct sk_buff *skb,
const struct dp_upcall_info *upcall_info)
{
struct ovs_header *upcall;
@@ -407,6 +408,7 @@ static int queue_userspace_packet(struct net *net, int dp_ifindex,
.snd_portid = upcall_info->portid,
};
size_t len;
+ unsigned int hlen;
int err;
if (vlan_tx_tag_present(skb)) {
@@ -427,7 +429,21 @@ static int queue_userspace_packet(struct net *net, int dp_ifindex,
goto out;
}
- len = upcall_msg_size(skb, upcall_info->userdata);
+ /* Complete checksum if needed */
+ if (skb->ip_summed == CHECKSUM_PARTIAL &&
+ (err = skb_checksum_help(skb)))
+ goto out;
+
+ /* Older versions of OVS user space enforce alignment of the last
+ * Netlink attribute to NLA_ALIGNTO which would require extensive
+ * padding logic. Only perform zerocopy if padding is not required.
+ */
+ if (dp->user_features & OVS_DP_F_UNALIGNED)
+ hlen = skb_zerocopy_headlen(skb);
+ else
+ hlen = skb->len;
+
+ len = upcall_msg_size(skb, upcall_info->userdata, hlen);
user_skb = genlmsg_new_unicast(len, &info, GFP_ATOMIC);
if (!user_skb) {
err = -ENOMEM;
@@ -447,13 +463,17 @@ static int queue_userspace_packet(struct net *net, int dp_ifindex,
nla_len(upcall_info->userdata),
nla_data(upcall_info->userdata));
- nla = __nla_reserve(user_skb, OVS_PACKET_ATTR_PACKET, skb->len);
+ /* Only reserve room for attribute header, packet data is added
+ * in skb_zerocopy() */
+ if (!(nla = nla_reserve(user_skb, OVS_PACKET_ATTR_PACKET, 0)))
+ goto out;
+ nla->nla_len = nla_attr_size(skb->len);
+
+ skb_zerocopy(user_skb, skb, skb->len, hlen);
- skb_copy_and_csum_dev(skb, nla_data(nla));
+ ((struct nlmsghdr *) user_skb->data)->nlmsg_len = user_skb->len;
- genlmsg_end(user_skb, upcall);
err = genlmsg_unicast(net, user_skb, upcall_info->portid);
-
out:
kfree_skb(nskb);
return err;
--
1.8.3.1
^ permalink raw reply related
* [PATCH openvswitch] linux: Signal datapath that unaligned Netlink message can be received
From: Thomas Graf @ 2013-11-21 18:15 UTC (permalink / raw)
To: jesse; +Cc: dev, netdev
Following commit (''netlink: Do not enforce alignment of last Netlink
attribute''), signal the ability to receive unaligned Netlink messages
to the datapath to enable utilization of zerocopy optimizations.
Signed-off-by: Thomas Graf <tgraf@redhat.com>
---
include/linux/openvswitch.h | 15 ++++++++++++++-
lib/dpif-linux.c | 6 ++++++
2 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/include/linux/openvswitch.h b/include/linux/openvswitch.h
index b429201..52dde86 100644
--- a/include/linux/openvswitch.h
+++ b/include/linux/openvswitch.h
@@ -60,7 +60,16 @@ struct ovs_header {
#define OVS_DATAPATH_FAMILY "ovs_datapath"
#define OVS_DATAPATH_MCGROUP "ovs_datapath"
-#define OVS_DATAPATH_VERSION 0x1
+
+/**
+ * V2:
+ * - API users are expected to provide OVS_DP_ATTR_USER_FEATURES
+ * with NLM_F_REPLACE when creating the datapath.
+ */
+#define OVS_DATAPATH_VERSION 2
+
+/* First OVS datapath version to support features */
+#define OVS_DP_VER_FEATURES 2
enum ovs_datapath_cmd {
OVS_DP_CMD_UNSPEC,
@@ -95,6 +104,7 @@ enum ovs_datapath_attr {
OVS_DP_ATTR_UPCALL_PID, /* Netlink PID to receive upcalls */
OVS_DP_ATTR_STATS, /* struct ovs_dp_stats */
OVS_DP_ATTR_MEGAFLOW_STATS, /* struct ovs_dp_megaflow_stats */
+ OVS_DP_ATTR_USER_FEATURES, /* OVS_DP_F_* */
__OVS_DP_ATTR_MAX
};
@@ -126,6 +136,9 @@ struct ovs_vport_stats {
__u64 tx_dropped; /* no space available in linux */
};
+/* Allow last Netlink attribute to be unaligned */
+#define OVS_DP_F_UNALIGNED (1 << 0)
+
/* Fixed logical ports. */
#define OVSP_LOCAL ((__u32)0)
diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c
index 1dcf321..5c952bc 100644
--- a/lib/dpif-linux.c
+++ b/lib/dpif-linux.c
@@ -73,6 +73,7 @@ struct dpif_linux_dp {
/* Attributes. */
const char *name; /* OVS_DP_ATTR_NAME. */
const uint32_t *upcall_pid; /* OVS_DP_ATTR_UPCALL_PID. */
+ uint32_t user_features; /* OVS_DP_ATTR_USER_FEATURES */
struct ovs_dp_stats stats; /* OVS_DP_ATTR_STATS. */
struct ovs_dp_megaflow_stats megaflow_stats;
/* OVS_DP_ATTR_MEGAFLOW_STATS.*/
@@ -232,6 +233,7 @@ dpif_linux_open(const struct dpif_class *class OVS_UNUSED, const char *name,
dp_request.cmd = OVS_DP_CMD_GET;
}
dp_request.name = name;
+ dp_request.user_features |= OVS_DP_F_UNALIGNED;
error = dpif_linux_dp_transact(&dp_request, &dp, &buf);
if (error) {
return error;
@@ -1839,6 +1841,10 @@ dpif_linux_dp_to_ofpbuf(const struct dpif_linux_dp *dp, struct ofpbuf *buf)
nl_msg_put_u32(buf, OVS_DP_ATTR_UPCALL_PID, *dp->upcall_pid);
}
+ if (dp->user_features) {
+ nl_msg_put_u32(buf, OVS_DP_ATTR_USER_FEATURES, dp->user_features);
+ }
+
/* Skip OVS_DP_ATTR_STATS since we never have a reason to serialize it. */
}
--
1.8.3.1
^ permalink raw reply related
* [PATCH openvswitch] netlink: Implement & enable memory mapped netlink i/o
From: Thomas Graf @ 2013-11-21 18:16 UTC (permalink / raw)
To: jesse; +Cc: dev, netdev, dborkman, ffusco, fleitner, Cong Wang
Based on the initial patch by Cong Wang posted a couple of months
ago.
This is the user space counterpart needed for the kernel patch
'[PATCH net-next 3/8] openvswitch: Enable memory mapped Netlink i/o'
Allows the kernel to construct Netlink messages on memory mapped
buffers and thus avoids copying. The functionality is enabled on
sockets used for unicast traffic.
Further optimizations are possible by avoiding the copy into the
ofpbuf after reading.
Cc: Cong Wang <xiyou.wangcong@gmail.com>
Signed-off-by: Thomas Graf <tgraf@redhat.com>
---
AUTHORS | 1 +
lib/dpif-linux.c | 6 +-
lib/netdev-linux.c | 2 +-
lib/netlink-notifier.c | 2 +-
lib/netlink-protocol.h | 33 ++++++
lib/netlink-socket.c | 288 +++++++++++++++++++++++++++++++++++++++++++------
lib/netlink-socket.h | 2 +-
utilities/nlmon.c | 2 +-
8 files changed, 298 insertions(+), 38 deletions(-)
diff --git a/AUTHORS b/AUTHORS
index 76951e7..f48aa5d 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -22,6 +22,7 @@ Bryan Phillippe bp@toroki.com
Casey Barker crbarker@google.com
Chris Wright chrisw@sous-sol.org
Chuck Short zulcss@ubuntu.com
+Cong Wang amwang@redhat.com
Damien Millescamps damien.millescamps@6wind.com
Dan Carpenter dan.carpenter@oracle.com
Dan Wendlandt dan@nicira.com
diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c
index 0442f77..1dcf321 100644
--- a/lib/dpif-linux.c
+++ b/lib/dpif-linux.c
@@ -495,7 +495,7 @@ dpif_linux_port_add__(struct dpif *dpif_, struct netdev *netdev,
int error;
if (dpif->epoll_fd >= 0) {
- error = nl_sock_create(NETLINK_GENERIC, &sock);
+ error = nl_sock_create(NETLINK_GENERIC, &sock, true);
if (error) {
return error;
}
@@ -765,7 +765,7 @@ dpif_linux_port_poll(const struct dpif *dpif_, char **devnamep)
struct nl_sock *sock;
int error;
- error = nl_sock_create(NETLINK_GENERIC, &sock);
+ error = nl_sock_create(NETLINK_GENERIC, &sock, false);
if (error) {
return error;
}
@@ -1265,7 +1265,7 @@ dpif_linux_recv_set__(struct dpif *dpif_, bool enable)
uint32_t upcall_pid;
int error;
- error = nl_sock_create(NETLINK_GENERIC, &sock);
+ error = nl_sock_create(NETLINK_GENERIC, &sock, true);
if (error) {
return error;
}
diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index ae0e5a0..ea39d22 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -478,7 +478,7 @@ netdev_linux_notify_sock(void)
if (ovsthread_once_start(&once)) {
int error;
- error = nl_sock_create(NETLINK_ROUTE, &sock);
+ error = nl_sock_create(NETLINK_ROUTE, &sock, false);
if (!error) {
error = nl_sock_join_mcgroup(sock, RTNLGRP_LINK);
if (error) {
diff --git a/lib/netlink-notifier.c b/lib/netlink-notifier.c
index 9aa185d..047ce75 100644
--- a/lib/netlink-notifier.c
+++ b/lib/netlink-notifier.c
@@ -109,7 +109,7 @@ nln_notifier_create(struct nln *nln, nln_notify_func *cb, void *aux)
struct nl_sock *sock;
int error;
- error = nl_sock_create(nln->protocol, &sock);
+ error = nl_sock_create(nln->protocol, &sock, false);
if (!error) {
error = nl_sock_join_mcgroup(sock, nln->multicast_group);
}
diff --git a/lib/netlink-protocol.h b/lib/netlink-protocol.h
index 3009fc5..1d18e22 100644
--- a/lib/netlink-protocol.h
+++ b/lib/netlink-protocol.h
@@ -145,6 +145,39 @@ enum {
};
#define CTRL_ATTR_OP_MAX (__CTRL_ATTR_OP_MAX - 1)
+
+#define NETLINK_RX_RING 6
+#define NETLINK_TX_RING 7
+
+struct nl_mmap_req {
+ unsigned int nm_block_size;
+ unsigned int nm_block_nr;
+ unsigned int nm_frame_size;
+ unsigned int nm_frame_nr;
+};
+
+struct nl_mmap_hdr {
+ unsigned int nm_status;
+ unsigned int nm_len;
+ __u32 nm_group;
+ /* credentials */
+ __u32 nm_pid;
+ __u32 nm_uid;
+ __u32 nm_gid;
+};
+
+enum nl_mmap_status {
+ NL_MMAP_STATUS_UNUSED,
+ NL_MMAP_STATUS_RESERVED,
+ NL_MMAP_STATUS_VALID,
+ NL_MMAP_STATUS_COPY,
+ NL_MMAP_STATUS_SKIP,
+};
+
+#define NL_MMAP_MSG_ALIGNMENT NLMSG_ALIGNTO
+#define NL_MMAP_MSG_ALIGN(sz) __ALIGN_KERNEL(sz, NL_MMAP_MSG_ALIGNMENT)
+#define NL_MMAP_HDRLEN NL_MMAP_MSG_ALIGN(sizeof(struct nl_mmap_hdr))
+
#endif /* !HAVE_NETLINK */
/* These were introduced all together in 2.6.24. */
diff --git a/lib/netlink-socket.c b/lib/netlink-socket.c
index 3420503..7181dfc 100644
--- a/lib/netlink-socket.c
+++ b/lib/netlink-socket.c
@@ -21,6 +21,7 @@
#include <stdlib.h>
#include <sys/types.h>
#include <sys/uio.h>
+#include <sys/mman.h>
#include <unistd.h>
#include "coverage.h"
#include "dynamic-string.h"
@@ -40,7 +41,9 @@ VLOG_DEFINE_THIS_MODULE(netlink_socket);
COVERAGE_DEFINE(netlink_overflow);
COVERAGE_DEFINE(netlink_received);
COVERAGE_DEFINE(netlink_recv_jumbo);
+COVERAGE_DEFINE(netlink_recv_mmap);
COVERAGE_DEFINE(netlink_sent);
+COVERAGE_DEFINE(netlink_sent_mmap);
/* Linux header file confusion causes this to be undefined. */
#ifndef SOL_NETLINK
@@ -58,12 +61,22 @@ static void log_nlmsg(const char *function, int error,
\f
/* Netlink sockets. */
+struct nl_ring {
+ unsigned int head;
+ void *ring;
+};
+
struct nl_sock {
int fd;
uint32_t next_seq;
uint32_t pid;
int protocol;
unsigned int rcvbuf; /* Receive buffer size (SO_RCVBUF). */
+ unsigned int frame_size;
+ unsigned int frame_nr;
+ size_t ring_size;
+ struct nl_ring tx_ring;
+ struct nl_ring rx_ring;
};
/* Compile-time limit on iovecs, so that we can allocate a maximum-size array
@@ -79,11 +92,51 @@ static int max_iovs;
static int nl_pool_alloc(int protocol, struct nl_sock **sockp);
static void nl_pool_release(struct nl_sock *);
+static int
+nl_sock_set_ring(struct nl_sock *sock)
+{
+ size_t block_size = 16 * getpagesize();
+ size_t ring_size;
+ void *ring;
+ struct nl_mmap_req req = {
+ .nm_block_size = block_size,
+ .nm_block_nr = 64,
+ .nm_frame_size = 16384,
+ };
+
+ req.nm_frame_nr = req.nm_block_nr * block_size / req.nm_frame_size;
+
+ if (setsockopt(sock->fd, SOL_NETLINK, NETLINK_RX_RING, &req, sizeof(req)) < 0
+ || setsockopt(sock->fd, SOL_NETLINK, NETLINK_TX_RING, &req, sizeof(req)) < 0) {
+ VLOG_INFO("mmap netlink is not supported");
+ return 0;
+ }
+
+
+ ring_size = req.nm_block_nr * req.nm_block_size;
+ ring = mmap(NULL, 2 * ring_size, PROT_READ | PROT_WRITE,
+ MAP_SHARED, sock->fd, 0);
+ if (ring == MAP_FAILED) {
+ VLOG_ERR("netlink mmap: %s", ovs_strerror(errno));
+ return errno;
+ }
+
+ sock->frame_size = req.nm_frame_size;
+ sock->frame_nr = req.nm_frame_nr - 1;
+ sock->ring_size = ring_size;
+ sock->rx_ring.ring = ring;
+ sock->rx_ring.head = 0;
+ sock->tx_ring.ring = (char *) ring + ring_size;
+ sock->tx_ring.head = 0;
+
+ return 0;
+}
+
/* Creates a new netlink socket for the given netlink 'protocol'
* (NETLINK_ROUTE, NETLINK_GENERIC, ...). Returns 0 and sets '*sockp' to the
* new socket if successful, otherwise returns a positive errno value. */
int
-nl_sock_create(int protocol, struct nl_sock **sockp)
+nl_sock_create(int protocol, struct nl_sock **sockp, bool use_mmap)
{
static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
struct nl_sock *sock;
@@ -120,6 +173,7 @@ nl_sock_create(int protocol, struct nl_sock **sockp)
}
sock->protocol = protocol;
sock->next_seq = 1;
+ sock->tx_ring.ring = sock->rx_ring.ring = NULL;
rcvbuf = 1024 * 1024;
if (setsockopt(sock->fd, SOL_SOCKET, SO_RCVBUFFORCE,
@@ -161,6 +215,11 @@ nl_sock_create(int protocol, struct nl_sock **sockp)
}
sock->pid = local.nl_pid;
+ if (use_mmap && (retval = nl_sock_set_ring(sock)) < 0) {
+ VLOG_ERR("failed to initialize memory mapped netlink socket");
+ goto error;
+ }
+
*sockp = sock;
return 0;
@@ -178,13 +237,19 @@ error:
return retval;
}
+static inline bool
+nl_sock_is_mapped(const struct nl_sock *sock)
+{
+ return sock->rx_ring.ring != NULL;
+}
+
/* Creates a new netlink socket for the same protocol as 'src'. Returns 0 and
* sets '*sockp' to the new socket if successful, otherwise returns a positive
* errno value. */
int
nl_sock_clone(const struct nl_sock *src, struct nl_sock **sockp)
{
- return nl_sock_create(src->protocol, sockp);
+ return nl_sock_create(src->protocol, sockp, nl_sock_is_mapped(src));
}
/* Destroys netlink socket 'sock'. */
@@ -192,6 +257,9 @@ void
nl_sock_destroy(struct nl_sock *sock)
{
if (sock) {
+ char *rx_ring = sock->rx_ring.ring;
+ if (rx_ring)
+ munmap(rx_ring, 2 * sock->ring_size);
close(sock->fd);
free(sock);
}
@@ -242,6 +310,95 @@ nl_sock_leave_mcgroup(struct nl_sock *sock, unsigned int multicast_group)
return 0;
}
+enum ring_type {
+ RX_RING,
+ TX_RING,
+};
+
+static struct nl_ring *
+mmap_ring(struct nl_sock *sock, enum ring_type ring)
+{
+ return ring == RX_RING ? &sock->rx_ring : &sock->tx_ring;
+}
+
+static struct nl_mmap_hdr *
+mmap_frame(struct nl_sock *sock, enum ring_type ring)
+{
+ struct nl_ring *r = mmap_ring(sock, ring);
+ char *start = r->ring;
+
+ return (struct nl_mmap_hdr *)(start + r->head * sock->frame_size);
+}
+
+static void
+mmap_advance_ring(struct nl_sock *sock, enum ring_type ring)
+{
+ struct nl_ring *r = mmap_ring(sock, ring);
+
+ if (r->head != sock->frame_nr) {
+ r->head++;
+ } else {
+ r->head = 0;
+ }
+}
+
+static int
+nl_sock_send_linear(struct nl_sock *sock, const struct ofpbuf *msg,
+ bool wait)
+{
+ int retval, error;
+
+ do {
+ retval = send(sock->fd, msg->data, msg->size, wait ? 0 : MSG_DONTWAIT);
+ error = retval < 0 ? errno : 0;
+ } while (error == EINTR);
+
+ return error;
+}
+
+static int
+nl_sock_send_mmap(struct nl_sock *sock, const struct ofpbuf *msg,
+ bool wait)
+{
+ struct nl_mmap_hdr *hdr;
+ struct sockaddr_nl addr = {
+ .nl_family = AF_NETLINK,
+ };
+ int retval, error;
+
+ if ((msg->size + NL_MMAP_HDRLEN) > sock->frame_size)
+ return nl_sock_send_linear(sock, msg, wait);
+
+ hdr = mmap_frame(sock, TX_RING);
+
+ if (hdr->nm_status != NL_MMAP_STATUS_UNUSED) {
+ /* No frame available. Block? */
+ if (wait) {
+ nl_sock_wait(sock, POLLOUT | POLLERR);
+ poll_block();
+ } else {
+ return EAGAIN;
+ }
+ }
+
+ memcpy((char *) hdr + NL_MMAP_HDRLEN, msg->data, msg->size);
+ hdr->nm_len = msg->size;
+ hdr->nm_status = NL_MMAP_STATUS_VALID;
+
+ mmap_advance_ring(sock, TX_RING);
+
+ do {
+ retval = sendto(sock->fd, NULL, 0, 0, (struct sockaddr *)&addr, sizeof(addr));
+ error = retval < 0 ? errno : 0;
+ } while (error == EINTR);
+
+ if (!error) {
+ COVERAGE_INC(netlink_sent_mmap);
+ }
+
+ return error;
+}
+
static int
nl_sock_send__(struct nl_sock *sock, const struct ofpbuf *msg,
uint32_t nlmsg_seq, bool wait)
@@ -252,11 +409,13 @@ nl_sock_send__(struct nl_sock *sock, const struct ofpbuf *msg,
nlmsg->nlmsg_len = msg->size;
nlmsg->nlmsg_seq = nlmsg_seq;
nlmsg->nlmsg_pid = sock->pid;
- do {
- int retval;
- retval = send(sock->fd, msg->data, msg->size, wait ? 0 : MSG_DONTWAIT);
- error = retval < 0 ? errno : 0;
- } while (error == EINTR);
+
+ if (sock->tx_ring.ring) {
+ error = nl_sock_send_mmap(sock, msg, wait);
+ } else {
+ error = nl_sock_send_linear(sock, msg, wait);
+ }
+
log_nlmsg(__func__, error, msg->data, msg->size, sock->protocol);
if (!error) {
COVERAGE_INC(netlink_sent);
@@ -297,26 +456,17 @@ nl_sock_send_seq(struct nl_sock *sock, const struct ofpbuf *msg,
}
static int
-nl_sock_recv__(struct nl_sock *sock, struct ofpbuf *buf, bool wait)
+nl_sock_recvmsg(struct nl_sock *sock, struct ofpbuf *buf, bool wait,
+ uint8_t *tail, size_t taillen)
{
- /* We can't accurately predict the size of the data to be received. The
- * caller is supposed to have allocated enough space in 'buf' to handle the
- * "typical" case. To handle exceptions, we make available enough space in
- * 'tail' to allow Netlink messages to be up to 64 kB long (a reasonable
- * figure since that's the maximum length of a Netlink attribute). */
- struct nlmsghdr *nlmsghdr;
- uint8_t tail[65536];
struct iovec iov[2];
struct msghdr msg;
- ssize_t retval;
-
- ovs_assert(buf->allocated >= sizeof *nlmsghdr);
- ofpbuf_clear(buf);
+ int retval;
iov[0].iov_base = buf->base;
iov[0].iov_len = buf->allocated;
iov[1].iov_base = tail;
- iov[1].iov_len = sizeof tail;
+ iov[1].iov_len = taillen;
memset(&msg, 0, sizeof msg);
msg.msg_iov = iov;
@@ -342,21 +492,97 @@ nl_sock_recv__(struct nl_sock *sock, struct ofpbuf *buf, bool wait)
return E2BIG;
}
- nlmsghdr = buf->data;
- if (retval < sizeof *nlmsghdr
- || nlmsghdr->nlmsg_len < sizeof *nlmsghdr
- || nlmsghdr->nlmsg_len > retval) {
- VLOG_ERR_RL(&rl, "received invalid nlmsg (%zd bytes < %zu)",
- retval, sizeof *nlmsghdr);
- return EPROTO;
- }
-
buf->size = MIN(retval, buf->allocated);
if (retval > buf->allocated) {
COVERAGE_INC(netlink_recv_jumbo);
ofpbuf_put(buf, tail, retval - buf->allocated);
}
+ return 0;
+}
+
+static int
+nl_sock_recv_mmap(struct nl_sock *sock, struct ofpbuf *buf, bool wait,
+ uint8_t *tail, size_t taillen)
+{
+ struct nl_mmap_hdr *hdr;
+ int retval = 0;
+
+restart:
+ hdr = mmap_frame(sock, RX_RING);
+
+ switch (hdr->nm_status) {
+ case NL_MMAP_STATUS_VALID:
+ if (hdr->nm_len == 0) {
+ /* error occured while constructing message */
+ hdr->nm_status = NL_MMAP_STATUS_UNUSED;
+ mmap_advance_ring(sock, RX_RING);
+ goto restart;
+ }
+
+ ofpbuf_put(buf, (char *) hdr + NL_MMAP_HDRLEN, hdr->nm_len);
+ COVERAGE_INC(netlink_recv_mmap);
+ break;
+
+ case NL_MMAP_STATUS_COPY:
+ retval = nl_sock_recvmsg(sock, buf, MSG_DONTWAIT, tail, taillen);
+ if (retval) {
+ return retval;
+ }
+ break;
+
+ case NL_MMAP_STATUS_UNUSED:
+ case NL_MMAP_STATUS_RESERVED:
+ default:
+ if (wait) {
+ nl_sock_wait(sock, POLLIN | POLLERR);
+ poll_block();
+ goto restart;
+ }
+
+ return EAGAIN;
+ }
+
+ hdr->nm_status = NL_MMAP_STATUS_UNUSED;
+ mmap_advance_ring(sock, RX_RING);
+
+ return retval;
+}
+
+static int
+nl_sock_recv__(struct nl_sock *sock, struct ofpbuf *buf, bool wait)
+{
+ /* We can't accurately predict the size of the data to be received. The
+ * caller is supposed to have allocated enough space in 'buf' to handle the
+ * "typical" case. To handle exceptions, we make available enough space in
+ * 'tail' to allow Netlink messages to be up to 64 kB long (a reasonable
+ * figure since that's the maximum length of a Netlink attribute). */
+ struct nlmsghdr *nlmsghdr;
+ uint8_t tail[65536];
+ int retval;
+
+ ovs_assert(buf->allocated >= sizeof *nlmsghdr);
+ ofpbuf_clear(buf);
+
+ if (sock->rx_ring.ring) {
+ retval = nl_sock_recv_mmap(sock, buf, wait, tail, sizeof(tail));
+ } else {
+ retval = nl_sock_recvmsg(sock, buf, wait, tail, sizeof(tail));
+ }
+
+ if (retval) {
+ return retval;
+ }
+
+ nlmsghdr = buf->data;
+ if (buf->size < sizeof *nlmsghdr
+ || nlmsghdr->nlmsg_len < sizeof *nlmsghdr
+ || nlmsghdr->nlmsg_len > buf->size) {
+ VLOG_ERR_RL(&rl, "received invalid nlmsg (%zd bytes < %zu)",
+ buf->size, sizeof *nlmsghdr);
+ return EPROTO;
+ }
+
log_nlmsg(__func__, 0, buf->data, buf->size, sock->protocol);
COVERAGE_INC(netlink_received);
@@ -892,7 +1118,7 @@ do_lookup_genl_family(const char *name, struct nlattr **attrs,
int error;
*replyp = NULL;
- error = nl_sock_create(NETLINK_GENERIC, &sock);
+ error = nl_sock_create(NETLINK_GENERIC, &sock, false);
if (error) {
return error;
}
@@ -1028,7 +1254,7 @@ nl_pool_alloc(int protocol, struct nl_sock **sockp)
*sockp = sock;
return 0;
} else {
- return nl_sock_create(protocol, sockp);
+ return nl_sock_create(protocol, sockp, true);
}
}
diff --git a/lib/netlink-socket.h b/lib/netlink-socket.h
index 18db417..c85fd64 100644
--- a/lib/netlink-socket.h
+++ b/lib/netlink-socket.h
@@ -50,7 +50,7 @@ struct nl_sock;
#endif
/* Netlink sockets. */
-int nl_sock_create(int protocol, struct nl_sock **);
+int nl_sock_create(int protocol, struct nl_sock **, bool);
int nl_sock_clone(const struct nl_sock *, struct nl_sock **);
void nl_sock_destroy(struct nl_sock *);
diff --git a/utilities/nlmon.c b/utilities/nlmon.c
index 99b060c..cc6bc68 100644
--- a/utilities/nlmon.c
+++ b/utilities/nlmon.c
@@ -47,7 +47,7 @@ main(int argc OVS_UNUSED, char *argv[])
set_program_name(argv[0]);
vlog_set_levels(NULL, VLF_ANY_FACILITY, VLL_DBG);
- error = nl_sock_create(NETLINK_ROUTE, &sock);
+ error = nl_sock_create(NETLINK_ROUTE, &sock, true);
if (error) {
ovs_fatal(error, "could not create rtnetlink socket");
}
--
1.8.3.1
^ permalink raw reply related
* Re: kernel BUG at net/core/skbuff.c:2839 RIP [<ffffffff819109a2>] skb_segment+0x6b2/0x6d0
From: David Miller @ 2013-11-21 18:17 UTC (permalink / raw)
To: eric.dumazet, herbert; +Cc: linux, edumazet, romieu, netdev
In-Reply-To: <1385043004.10637.34.camel@edumazet-glaptop2.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 21 Nov 2013 06:10:04 -0800
> As mentioned earlier, this problem is known and a patch is under review.
>
> http://marc.info/?l=linux-netdev&m=138419594024851&w=2
It seems this patch still requires some improvements, can you guys sort
out what needs to be done so that I can apply a fix soon?
Thanks!
^ permalink raw reply
* Re: [PATCHv1 net] xen-netback: stop the VIF thread before unbinding IRQs
From: David Miller @ 2013-11-21 18:17 UTC (permalink / raw)
To: david.vrabel
Cc: xen-devel, konrad.wilk, boris.ostrovsky, ian.campbell, wei.liu2,
netdev
In-Reply-To: <1385047569-23622-1-git-send-email-david.vrabel@citrix.com>
From: David Vrabel <david.vrabel@citrix.com>
Date: Thu, 21 Nov 2013 15:26:09 +0000
> From: David Vrabel <david.vrabel@citrix.com>
>
> If the VIF thread is still running after unbinding the Tx and Rx IRQs
> in xenvif_disconnect(), the thread may attempt to raise an event which
> will BUG (as the irq is unbound).
>
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
Applied.
^ permalink raw reply
* Re: [PATCH v3] packet: fix use after free race in send path when dev is released
From: David Miller @ 2013-11-21 18:18 UTC (permalink / raw)
To: dborkman; +Cc: netdev, noureddine, greearb, eric.dumazet
In-Reply-To: <1385049058-1946-1-git-send-email-dborkman@redhat.com>
From: Daniel Borkmann <dborkman@redhat.com>
Date: Thu, 21 Nov 2013 16:50:58 +0100
> Salam reported a use after free bug in PF_PACKET that occurs when
> we're sending out frames on a socket bound device and suddenly the
> net device is being unregistered. It appears that commit 827d9780
> introduced a possible race condition between {t,}packet_snd() and
> packet_notifier(). In the case of a bound socket, packet_notifier()
> can drop the last reference to the net_device and {t,}packet_snd()
> might end up suddenly sending a packet over a freed net_device.
>
> To avoid reverting 827d9780 and thus introducing a performance
> regression compared to the current state of things, we decided to
> hold a cached RCU protected pointer to the net device and maintain
> it on write side via bind spin_lock protected register_prot_hook()
> and __unregister_prot_hook() calls.
>
> In {t,}packet_snd() path, we access this pointer under rcu_read_lock
> through packet_cached_dev_get() that holds reference to the device
> to prevent it from being freed through packet_notifier() while
> we're in send path. This is okay to do as dev_put()/dev_hold() are
> per-cpu counters, so this should not be a performance issue. Also,
> the code simplifies a bit as we don't need need_rls_dev anymore.
>
> Fixes: 827d978037d7 ("af-packet: Use existing netdev reference for bound sockets.")
> Reported-by: Salam Noureddine <noureddine@aristanetworks.com>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> Signed-off-by: Salam Noureddine <noureddine@aristanetworks.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH] genetlink: fix genlmsg_multicast() bug
From: David Miller @ 2013-11-21 18:18 UTC (permalink / raw)
To: johannes; +Cc: netdev, johannes.berg
In-Reply-To: <1385054224-1410-1-git-send-email-johannes@sipsolutions.net>
From: Johannes Berg <johannes@sipsolutions.net>
Date: Thu, 21 Nov 2013 18:17:04 +0100
> From: Johannes Berg <johannes.berg@intel.com>
>
> Unfortunately, I introduced a tremendously stupid bug into
> genlmsg_multicast() when doing all those multicast group
> changes: it adjusts the group number, but then passes it
> to genlmsg_multicast_netns() which does that again.
>
> Somehow, my tests failed to catch this, so add a warning
> into genlmsg_multicast_netns() and remove the offending
> group ID adjustment.
>
> Also add a warning to the similar code in other functions
> so people who misuse them are more loudly warned.
>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Applied.
^ permalink raw reply
* Re: [PATCH] genetlink: fix genl_set_err() group ID
From: David Miller @ 2013-11-21 18:18 UTC (permalink / raw)
To: johannes; +Cc: netdev, johannes.berg
In-Reply-To: <1385054428-6447-1-git-send-email-johannes@sipsolutions.net>
From: Johannes Berg <johannes@sipsolutions.net>
Date: Thu, 21 Nov 2013 18:20:28 +0100
> From: Johannes Berg <johannes.berg@intel.com>
>
> Fix another really stupid bug - I introduced genl_set_err()
> precisely to be able to adjust the group and reject invalid
> ones, but then forgot to do so.
>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Applied.
^ permalink raw reply
* Re: [PATCH] wimax: remove dead code
From: David Miller @ 2013-11-21 18:19 UTC (permalink / raw)
To: johannes
Cc: joe, michael.opdenacker, inaky.perez-gonzalez, linux-wimax, wimax,
netdev, linux-kernel
In-Reply-To: <1385018850.14273.0.camel@jlt4.sipsolutions.net>
From: Johannes Berg <johannes@sipsolutions.net>
Date: Thu, 21 Nov 2013 08:27:30 +0100
> On Wed, 2013-11-20 at 20:17 -0800, Joe Perches wrote:
>> On Thu, 2013-11-21 at 04:43 +0100, Michael Opdenacker wrote:
>> > This removes a code line that is between a "return 0;" and an error label.
>> > This code line can never be reached.
>>
>> Adding Johannes Berg who added this oversight.
>
> Obviously, sorry about that. I seem to remember deleting *something*
> there (like another error label) but I guess I missed that line.
>
> Acked-by: Johannes Berg <johannes@sipsolutions.net>
Applied, thanks.
^ permalink raw reply
* Re:Answer back
From: lee hyuk @ 2013-11-21 12:53 UTC (permalink / raw)
I would like to discuss a very important crude oil project with you,kindly
revert back to me if this is your valid email address for further
information.
Regards,
Lee
^ permalink raw reply
* Re: [PATCH net-next 7/8] openvswitch: Drop user features if old user space attempted to create datapath
From: Ben Hutchings @ 2013-11-21 18:23 UTC (permalink / raw)
To: Thomas Graf
Cc: jesse, davem, dev, netdev, dborkman, ffusco, fleitner,
eric.dumazet
In-Reply-To: <68ddfb6e0ef4401f246f2599ade5c15b25063736.1385057355.git.tgraf@suug.ch>
On Thu, 2013-11-21 at 19:13 +0100, Thomas Graf wrote:
> Drop user features if an outdated user space instance that does not
> understand the concept of user_features attempted to create a new
> datapath.
>
> Signed-off-by: Thomas Graf <tgraf@suug.ch>
> Reviewed-by: Daniel Borkmann <dborkman@redhat.com>
> ---
> include/uapi/linux/openvswitch.h | 11 ++++++++++-
> net/openvswitch/datapath.c | 10 ++++++++++
> 2 files changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
> index 07ef2c3..4f74c2c 100644
> --- a/include/uapi/linux/openvswitch.h
> +++ b/include/uapi/linux/openvswitch.h
> @@ -40,7 +40,16 @@ struct ovs_header {
>
> #define OVS_DATAPATH_FAMILY "ovs_datapath"
> #define OVS_DATAPATH_MCGROUP "ovs_datapath"
> -#define OVS_DATAPATH_VERSION 0x1
> +
> +/**
> + * V2:
This is not kernel-doc format so don't use '/**'.
> + * - API users are expected to provide OVS_DP_ATTR_USER_FEATURES
> + * with NLM_F_REPLACE when creating the datapath.
> + */
> +#define OVS_DATAPATH_VERSION 2
> +
> +/* First OVS datapath version to support features */
> +#define OVS_DP_VER_FEATURES 2
>
> enum ovs_datapath_cmd {
> OVS_DP_CMD_UNSPEC,
> diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
> index 3f1fb87..72cdffb 100644
> --- a/net/openvswitch/datapath.c
> +++ b/net/openvswitch/datapath.c
> @@ -1198,6 +1198,16 @@ static int ovs_dp_create_or_update(struct sk_buff *skb, struct genl_info *info,
> if (info->nlhdr->nlmsg_flags & NLM_F_REPLACE)
> goto update;
>
> + /* An outdated user space instance that does not understand
> + * the concept of user_features has attempted to create a new
> + * datapath. Drop all user features.
> + */
> + if (info->genlhdr->version < OVS_DP_VER_FEATURES) {
> + WARN_ONCE(dp->user_features, "Dropping previously "
> + "announced user features");
Log messages shouldn't be split like this as it makes them harder to
find. There should also be a newline at the end of the message.
Ben.
> + dp->user_features = 0;
> + }
> +
> err = -EEXIST;
> goto err_unlock_ovs;
> }
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: kernel BUG at net/core/skbuff.c:2839 RIP [<ffffffff819109a2>] skb_segment+0x6b2/0x6d0
From: Eric Dumazet @ 2013-11-21 18:23 UTC (permalink / raw)
To: David Miller; +Cc: herbert, linux, edumazet, romieu, netdev
In-Reply-To: <20131121.131741.737292941735802990.davem@davemloft.net>
On Thu, 2013-11-21 at 13:17 -0500, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Thu, 21 Nov 2013 06:10:04 -0800
>
> > As mentioned earlier, this problem is known and a patch is under review.
> >
> > http://marc.info/?l=linux-netdev&m=138419594024851&w=2
>
> It seems this patch still requires some improvements, can you guys sort
> out what needs to be done so that I can apply a fix soon?
I think we might apply this patch as is, and do the TSO optimization
when net-next reopens ?
I was planning to resend Herbert patch today anyway, after a new round
of testing on my hosts.
^ permalink raw reply
* Re: [PATCH v4 net-next] net: introduce dev_set_forwarding()
From: David Miller @ 2013-11-21 18:29 UTC (permalink / raw)
To: eric.dumazet
Cc: bhutchings, christoph.paasch, herbert, netdev, hkchu, mwdalton
In-Reply-To: <1383422330.4291.58.camel@edumazet-glaptop2.roam.corp.google.com>
Eric please resubmit this in the future once all of the other fixes
have been sorted out with Herbert.
Thanks.
^ permalink raw reply
* Re: [PATCH v3] net: Do not include padding in TCP GRO checksum
From: David Miller @ 2013-11-21 18:35 UTC (permalink / raw)
To: alexander.h.duyck; +Cc: herbert, alexander.duyck, netdev, edumazet
In-Reply-To: <528A51B6.2040609@intel.com>
From: Alexander Duyck <alexander.h.duyck@intel.com>
Date: Mon, 18 Nov 2013 09:43:18 -0800
> I'm not going to have a chance to test this today, but on review it
> should fix the issue.
>
> Acked-by: Alexander Duyck <alexander.h.duyck@intel.com>
I've lost track of this dicussion, Herbert could you post the patches
I should apply? I think it was this one and a follow-on simplification
to the checksum handling?
Thanks.
^ permalink raw reply
* RE: [PATCH net v3] vti: fix spd lookup: match plaintext pkt, not ipsec pkt
From: Saurabh Mohan @ 2013-11-21 18:35 UTC (permalink / raw)
To: Steffen Klassert, Christophe Gouault,
Fan Du (fan.du@windriver.com)
Cc: David S. Miller, Herbert Xu, netdev@vger.kernel.org,
Sergei Shtylyov, Eric Dumazet
In-Reply-To: <20131121121246.GD31491@secunet.com>
> -----Original Message-----
> From: Steffen Klassert [mailto:steffen.klassert@secunet.com]
> Sent: Thursday, November 21, 2013 4:13 AM
> To: Christophe Gouault
> Cc: David S. Miller; Herbert Xu; netdev@vger.kernel.org; Saurabh Mohan;
> Sergei Shtylyov; Eric Dumazet
> Subject: Re: [PATCH net v3] vti: fix spd lookup: match plaintext pkt, not ipsec
> pkt
>
> On Wed, Nov 06, 2013 at 09:05:53AM +0100, Christophe Gouault wrote:
> >
> > @@ -133,7 +134,13 @@ static int vti_rcv(struct sk_buff *skb)
> > * only match policies with this mark.
> > */
> > skb->mark = be32_to_cpu(tunnel->parms.o_key);
> > + /* The packet is decrypted, but not yet decapsulated.
> > + * Temporarily make network_header point to the inner
> header
> > + * for policy check.
> > + */
> > + skb_reset_network_header(skb);
> > ret = xfrm4_policy_check(NULL, XFRM_POLICY_IN, skb);
>
> If we do it like this, we would do an input policy check even for
> packets that should be forwarded. I think that's a bit odd.
>
> If we really change to match plaintext traffic, we should do
> it like Fan Du proposed. Remove the policy check here and
> let the further layers do the policy enforcement. All we
> have to do is to set the skb mark, then the lookup should
> match the vti policy.
>
> It is already clear that this packet was IPsec transformed
> when it enters vti_rcv, so deferring the policy check should
> be ok.
Agreed. The right thing to do here is apply the vti mark
and then let xfrm policy check match the clear packet with
the vti-mark on it.
The side-effect is that we'll lose the old mark.
^ permalink raw reply
* Re: kernel BUG at net/core/skbuff.c:2839 RIP [<ffffffff819109a2>] skb_segment+0x6b2/0x6d0
From: David Miller @ 2013-11-21 18:38 UTC (permalink / raw)
To: eric.dumazet; +Cc: herbert, linux, edumazet, romieu, netdev
In-Reply-To: <1385058216.10637.53.camel@edumazet-glaptop2.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 21 Nov 2013 10:23:36 -0800
> I was planning to resend Herbert patch today anyway, after a new round
> of testing on my hosts.
Please do, it would be nice to have your tested-by or similar tag on
it at least.
Thanks!
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox