* Re: 3.13-rc1: eth0 hw csum failure on powerpc
From: Christian Kujau @ 2013-11-30 10:11 UTC (permalink / raw)
To: LKML; +Cc: netdev, linuxppc-dev
In-Reply-To: <alpine.DEB.2.11.1311300156380.31521@trent.utfs.org>
On Sat, 30 Nov 2013 at 02:07, Christian Kujau wrote:
> while trying to upgrade from 3.12.0 to 3.13-rc1 or -rc2, the following
> happens early during bootup:
>
> [...]
> Freeing unused kernel memory: 204K (c06ea000 - c071d000)
> eth0: hw csum failure
> CPU: 0 PID: 0 Comm: swapper Not tainted 3.13.0-rc2 #1
> Call Trace:
Btw, full dmesg & .config: http://nerdbynature.de/bits/3.13-rc1/
C.
--
BOFH excuse #449:
greenpeace free'd the mallocs
^ permalink raw reply
* [PATCH] sctp: check the rto_min and rto_max
From: wangweidong @ 2013-11-30 10:23 UTC (permalink / raw)
To: Vlad Yasevich, nhorman, davem; +Cc: linux-sctp, netdev, dingtianhong
rto_min should be smaller than rto_max while rto_max should be larger
than rto_min. so just add the check.
Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
---
net/sctp/sysctl.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/sctp/sysctl.c b/net/sctp/sysctl.c
index 6b36561..7637e8e 100644
--- a/net/sctp/sysctl.c
+++ b/net/sctp/sysctl.c
@@ -104,7 +104,7 @@ static struct ctl_table sctp_net_table[] = {
.mode = 0644,
.proc_handler = proc_dointvec_minmax,
.extra1 = &one,
- .extra2 = &timer_max
+ .extra2 = &init_net.sctp.rto_max
},
{
.procname = "rto_max",
@@ -112,7 +112,7 @@ static struct ctl_table sctp_net_table[] = {
.maxlen = sizeof(unsigned int),
.mode = 0644,
.proc_handler = proc_dointvec_minmax,
- .extra1 = &one,
+ .extra1 = &init_net.sctp.rto_min,
.extra2 = &timer_max
},
{
--
1.7.12
^ permalink raw reply related
* Re: [PATCH] net: mac80211: tx.c: be sure of 'sdata->vif.type' must be NL80211_IFTYPE_AP when be in NL80211_IFTYPE_AP case
From: Chen Gang @ 2013-11-30 11:59 UTC (permalink / raw)
To: Johannes Berg
Cc: John W. Linville, rkuo, linux-kernel@vger.kernel.org,
David Miller, linux-wireless, netdev
In-Reply-To: <1385739487.8656.1.camel@jlt4.sipsolutions.net>
On 11/29/2013 11:38 PM, Johannes Berg wrote:
>
>> +++ b/net/mac80211/tx.c
>> @@ -1814,8 +1814,9 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
>> break;
>> /* fall through */
>> case NL80211_IFTYPE_AP:
>> - if (sdata->vif.type == NL80211_IFTYPE_AP)
>> - chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
>> + if (sdata->vif.type != NL80211_IFTYPE_AP)
>> + goto fail_rcu;
>> + chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
>
> This change is completely wrong.
>
Oh, it is.
Hmm... for me, this work flow still can be implemented with a little
clearer way (at least it will avoid related warning):
-------------------------diff begin------------------------------
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index c558b24..7076128 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1810,14 +1810,14 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
if (!chanctx_conf)
goto fail_rcu;
band = chanctx_conf->def.chan->band;
- if (sta)
- break;
- /* fall through */
+ if (!sta)
+ goto try_next;
+ break;
case NL80211_IFTYPE_AP:
- if (sdata->vif.type == NL80211_IFTYPE_AP)
- chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
+ chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
if (!chanctx_conf)
goto fail_rcu;
+try_next:
fc |= cpu_to_le16(IEEE80211_FCTL_FROMDS);
/* DA BSSID SA */
memcpy(hdr.addr1, skb->data, ETH_ALEN);
-------------------------diff end--------------------------------
Thanks.
--
Chen Gang
^ permalink raw reply related
* [net-next 2/7] netlink: Avoid netlink mmap alloc if msg size exceeds frame size
From: Thomas Graf @ 2013-11-30 12:21 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.1385813891.git.tgraf-G/eBtMaohhA@public.gmane.org>
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-G/eBtMaohhA@public.gmane.org>
Reviewed-by: Daniel Borkmann <dborkman-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
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 0/8 v8] Open vSwitch upcall optimiziations
From: Thomas Graf @ 2013-11-30 12:21 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.
V8: - Dropped patch adding NLM_F_REPLACE support, I'll pursue this in a
separate patch series. Addresses Jesse's comments.
- Improved comment describing OVS_DATAPATH_VERSION bump.
V7: - removed unintential kernel-doc comment
- WARN_ONCE() -> WARN(), message on single line, added \n
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 (7):
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: 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 | 14 +++-
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 | 143 ++++++++++++++++++++++++-----------
net/openvswitch/datapath.h | 2 +
9 files changed, 236 insertions(+), 99 deletions(-)
--
1.8.3.1
^ permalink raw reply
* [net-next 3/7] openvswitch: Enable memory mapped Netlink i/o
From: Thomas Graf @ 2013-11-30 12:21 UTC (permalink / raw)
To: jesse, davem
Cc: dev, netdev, dborkman, ffusco, fleitner, eric.dumazet, bhutchings
In-Reply-To: <cover.1385813891.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
* [net-next 4/7] net: Export skb_zerocopy() to zerocopy from one skb to another
From: Thomas Graf @ 2013-11-30 12:21 UTC (permalink / raw)
To: jesse, davem
Cc: dev, netdev, dborkman, ffusco, fleitner, eric.dumazet, bhutchings
In-Reply-To: <cover.1385813891.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 2718fed..55859cb 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
* [net-next 1/7] genl: Add genlmsg_new_unicast() for unicast message allocation
From: Thomas Graf @ 2013-11-30 12:21 UTC (permalink / raw)
To: jesse, davem
Cc: dev, netdev, dborkman, ffusco, fleitner, eric.dumazet, bhutchings
In-Reply-To: <cover.1385813891.git.tgraf@suug.ch>
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@suug.ch>
Reviewed-by: Daniel Borkmann <dborkman@redhat.com>
---
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 1b177ed..93695f0 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 713671a..b1dcdb9 100644
--- a/net/netlink/genetlink.c
+++ b/net/netlink/genetlink.c
@@ -461,6 +461,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
@@ -600,6 +620,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
* [net-next 5/7] openvswitch: Allow user space to announce ability to accept unaligned Netlink messages
From: Thomas Graf @ 2013-11-30 12:21 UTC (permalink / raw)
To: jesse, davem
Cc: dev, netdev, dborkman, ffusco, fleitner, eric.dumazet, bhutchings
In-Reply-To: <cover.1385813891.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
* [net-next 6/7] openvswitch: Drop user features if old user space attempted to create datapath
From: Thomas Graf @ 2013-11-30 12:21 UTC (permalink / raw)
To: jesse, davem
Cc: dev, netdev, dborkman, ffusco, fleitner, eric.dumazet, bhutchings
In-Reply-To: <cover.1385813891.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 | 10 +++++++++-
net/openvswitch/datapath.c | 21 +++++++++++++++++++++
2 files changed, 30 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
index 07ef2c3..970553c 100644
--- a/include/uapi/linux/openvswitch.h
+++ b/include/uapi/linux/openvswitch.h
@@ -40,7 +40,15 @@ 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
+ * 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 95d4424..8eaa39a 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -1175,6 +1175,18 @@ static struct datapath *lookup_datapath(struct net *net,
return dp ? dp : ERR_PTR(-ENODEV);
}
+static void ovs_dp_reset_user_features(struct sk_buff *skb, struct genl_info *info)
+{
+ struct datapath *dp;
+
+ dp = lookup_datapath(sock_net(skb->sk), info->userhdr, info->attrs);
+ if (!dp)
+ return;
+
+ WARN(dp->user_features, "Dropping previously announced user features\n");
+ dp->user_features = 0;
+}
+
static void ovs_dp_change(struct datapath *dp, struct nlattr **a)
{
if (a[OVS_DP_ATTR_USER_FEATURES])
@@ -1247,6 +1259,15 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct genl_info *info)
if (err == -EBUSY)
err = -EEXIST;
+ if (err == -EEXIST) {
+ /* An outdated user space instance that does not understand
+ * the concept of user_features has attempted to create a new
+ * datapath and is likely to reuse it. Drop all user features.
+ */
+ if (info->genlhdr->version < OVS_DP_VER_FEATURES)
+ ovs_dp_reset_user_features(skb, info);
+ }
+
goto err_destroy_ports_array;
}
--
1.8.3.1
^ permalink raw reply related
* [net-next 7/7] openvswitch: Use skb_zerocopy() for upcall
From: Thomas Graf @ 2013-11-30 12:21 UTC (permalink / raw)
To: jesse, davem
Cc: dev, netdev, dborkman, ffusco, fleitner, eric.dumazet, bhutchings
In-Reply-To: <cover.1385813891.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 8eaa39a..867edf1 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 v2] linux: Signal datapath that unaligned Netlink message can be received
From: Thomas Graf @ 2013-11-30 12:25 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>
---
V2: - Only provide OVS_DP_ATTR_USER_FEATURES on OVS_DP_CMD_NEW
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..80266b1 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
+ * when creating or updating 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 6c482d0..2d8a1aa 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.*/
@@ -228,6 +229,7 @@ dpif_linux_open(const struct dpif_class *class OVS_UNUSED, const char *name,
dp_request.cmd = OVS_DP_CMD_NEW;
upcall_pid = 0;
dp_request.upcall_pid = &upcall_pid;
+ dp_request.user_features |= OVS_DP_F_UNALIGNED;
} else {
dp_request.cmd = OVS_DP_CMD_GET;
}
@@ -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 v2] netlink: Implement & enable memory mapped netlink i/o
From: Thomas Graf @ 2013-11-30 12:27 UTC (permalink / raw)
To: jesse, blp; +Cc: dev, netdev, dborkman, ffusco, fleitner, xiyou.wangcong
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.
Signed-off-by: Thomas Graf <tgraf@redhat.com>
---
V2: - Provide required definitions in netlink-protocol.h if <linux/netlink.h>
does not contain them.
AUTHORS | 1 +
lib/dpif-linux.c | 6 +-
lib/netdev-linux.c | 2 +-
lib/netlink-notifier.c | 2 +-
lib/netlink-protocol.h | 34 ++++++
lib/netlink-socket.c | 288 +++++++++++++++++++++++++++++++++++++++++++------
lib/netlink-socket.h | 2 +-
utilities/nlmon.c | 2 +-
8 files changed, 299 insertions(+), 38 deletions(-)
diff --git a/AUTHORS b/AUTHORS
index 1c2d9ea..4d86f86 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -23,6 +23,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 25715f4..6c482d0 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 3e0da48..3bb4618 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..9009711 100644
--- a/lib/netlink-protocol.h
+++ b/lib/netlink-protocol.h
@@ -179,4 +179,38 @@ enum {
#define CTRL_ATTR_MCAST_GRP_MAX (__CTRL_ATTR_MCAST_GRP_MAX - 1)
#endif /* CTRL_ATTR_MCAST_GRP_MAX */
+#ifndef NETLINK_RX_RING
+#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 /* NETLINK_RX_RING */
+
#endif /* netlink-protocol.h */
diff --git a/lib/netlink-socket.c b/lib/netlink-socket.c
index 4bd6d36..3141ab6 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 (%"PRIuSIZE"d bytes < %"PRIuSIZE")",
- 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 (%"PRIuSIZE"d bytes < %"PRIuSIZE")",
+ 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: [net-next 3/7] openvswitch: Enable memory mapped Netlink i/o
From: Florian Westphal @ 2013-11-30 12:35 UTC (permalink / raw)
To: Thomas Graf
Cc: dev-yBygre7rU0TnMu66kgdUjQ, fleitner-H+wXaHxf7aLQT0dZR+AlfA,
eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA, bhutchings-s/n/eUQHGBpZroRs9YW3xA,
davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <9f7f05a726935c434d43d92c2908a997c403725b.1385813891.git.tgraf-G/eBtMaohhA@public.gmane.org>
Thomas Graf <tgraf-G/eBtMaohhA@public.gmane.org> wrote:
> 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
[..]
> After:
> Result: OK: 24229690(c24127165+d102524) usec, 5000000 (9000byte,0frags)
> 206358pps 14857Mb/sec (14857776000bps) errors: 0
I'm curious. Is the 'old' value with skb_zerocopy() or without?
Does ovs-vswitchd 'read-access' the entire packet or just e.g. the
header?
I ask because in netfilter nfqueue tests I could not see any difference
between 'zerocopy' vs. mmap in the receive-path tests I made a while
back.
[ I have no objections to your patch, of course ].
^ permalink raw reply
* Re: [PATCH] net: mac80211: tx.c: be sure of 'sdata->vif.type' must be NL80211_IFTYPE_AP when be in NL80211_IFTYPE_AP case
From: Johannes Berg @ 2013-11-30 12:53 UTC (permalink / raw)
To: Chen Gang
Cc: John W. Linville, rkuo, linux-kernel@vger.kernel.org,
David Miller, linux-wireless, netdev
In-Reply-To: <5299D306.7070701@gmail.com>
On Sat, 2013-11-30 at 19:59 +0800, Chen Gang wrote:
> On 11/29/2013 11:38 PM, Johannes Berg wrote:
> >
> >> +++ b/net/mac80211/tx.c
> >> @@ -1814,8 +1814,9 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
> >> break;
> >> /* fall through */
> >> case NL80211_IFTYPE_AP:
> >> - if (sdata->vif.type == NL80211_IFTYPE_AP)
> >> - chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
> >> + if (sdata->vif.type != NL80211_IFTYPE_AP)
> >> + goto fail_rcu;
> >> + chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
> >
> > This change is completely wrong.
> >
>
> Oh, it is.
>
> Hmm... for me, this work flow still can be implemented with a little
> clearer way (at least it will avoid related warning):
>
> -------------------------diff begin------------------------------
>
> diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
> index c558b24..7076128 100644
> --- a/net/mac80211/tx.c
> +++ b/net/mac80211/tx.c
> @@ -1810,14 +1810,14 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
> if (!chanctx_conf)
> goto fail_rcu;
> band = chanctx_conf->def.chan->band;
> - if (sta)
> - break;
> - /* fall through */
> + if (!sta)
> + goto try_next;
> + break;
> case NL80211_IFTYPE_AP:
> - if (sdata->vif.type == NL80211_IFTYPE_AP)
> - chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
> + chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
> if (!chanctx_conf)
> goto fail_rcu;
> +try_next:
I don't think that's better than the (fairly obvious) fall-through, and
has a pretty odd goto. Also, depending on the compiler, it still knows
the previous case label and doesn't warn.
johannes
^ permalink raw reply
* Re: [PATCH] net: mac80211: tx.c: be sure of 'sdata->vif.type' must be NL80211_IFTYPE_AP when be in NL80211_IFTYPE_AP case
From: Chen Gang @ 2013-11-30 13:50 UTC (permalink / raw)
To: Johannes Berg
Cc: John W. Linville, rkuo,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
David Miller, linux-wireless-u79uwXL29TY76Z2rM5mHXA, netdev
In-Reply-To: <1385816013.4327.1.camel-8Nb76shvtaUJvtFkdXX2HixXY32XiHfO@public.gmane.org>
On 11/30/2013 08:53 PM, Johannes Berg wrote:
> On Sat, 2013-11-30 at 19:59 +0800, Chen Gang wrote:
>> On 11/29/2013 11:38 PM, Johannes Berg wrote:
>>>
>>>> +++ b/net/mac80211/tx.c
>>>> @@ -1814,8 +1814,9 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
>>>> break;
>>>> /* fall through */
>>>> case NL80211_IFTYPE_AP:
>>>> - if (sdata->vif.type == NL80211_IFTYPE_AP)
>>>> - chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
>>>> + if (sdata->vif.type != NL80211_IFTYPE_AP)
>>>> + goto fail_rcu;
>>>> + chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
>>>
>>> This change is completely wrong.
>>>
>>
>> Oh, it is.
>>
>> Hmm... for me, this work flow still can be implemented with a little
>> clearer way (at least it will avoid related warning):
>>
>> -------------------------diff begin------------------------------
>>
>> diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
>> index c558b24..7076128 100644
>> --- a/net/mac80211/tx.c
>> +++ b/net/mac80211/tx.c
>> @@ -1810,14 +1810,14 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
>> if (!chanctx_conf)
>> goto fail_rcu;
>> band = chanctx_conf->def.chan->band;
>> - if (sta)
>> - break;
>> - /* fall through */
>> + if (!sta)
>> + goto try_next;
>> + break;
>> case NL80211_IFTYPE_AP:
>> - if (sdata->vif.type == NL80211_IFTYPE_AP)
>> - chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
>> + chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
>> if (!chanctx_conf)
>> goto fail_rcu;
>> +try_next:
>
> I don't think that's better than the (fairly obvious) fall-through, and
> has a pretty odd goto. Also, depending on the compiler, it still knows
> the previous case label and doesn't warn.
>
Yeah, fall-through is obvious. But check 'A' again just near by "case A"
seems a little strange, and some of compilers (or some of versions) are
really not quit smart enough to know it is not a warning.
Hmm... for me, if the code (implementation) can express real logical
work flow as much as directly and simply, the code (implementation) is
clear enough (don't mind whether use 'goto' or not).
And originally, at first, I am really not quite careful enough, and sent
an incorrect patch after noticed the related compiler's warning. :-)
Thanks.
--
Chen Gang
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] net: mac80211: tx.c: be sure of 'sdata->vif.type' must be NL80211_IFTYPE_AP when be in NL80211_IFTYPE_AP case
From: Chen Gang @ 2013-11-30 14:02 UTC (permalink / raw)
To: Johannes Berg
Cc: John W. Linville, rkuo, linux-kernel@vger.kernel.org,
David Miller, linux-wireless, netdev
In-Reply-To: <5299ED38.4090509@gmail.com>
On 11/30/2013 09:50 PM, Chen Gang wrote:
> On 11/30/2013 08:53 PM, Johannes Berg wrote:
>> On Sat, 2013-11-30 at 19:59 +0800, Chen Gang wrote:
>>> On 11/29/2013 11:38 PM, Johannes Berg wrote:
>>>>
>>>>> +++ b/net/mac80211/tx.c
>>>>> @@ -1814,8 +1814,9 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
>>>>> break;
>>>>> /* fall through */
>>>>> case NL80211_IFTYPE_AP:
>>>>> - if (sdata->vif.type == NL80211_IFTYPE_AP)
>>>>> - chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
>>>>> + if (sdata->vif.type != NL80211_IFTYPE_AP)
>>>>> + goto fail_rcu;
>>>>> + chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
>>>>
>>>> This change is completely wrong.
>>>>
>>>
>>> Oh, it is.
>>>
>>> Hmm... for me, this work flow still can be implemented with a little
>>> clearer way (at least it will avoid related warning):
>>>
>>> -------------------------diff begin------------------------------
>>>
>>> diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
>>> index c558b24..7076128 100644
>>> --- a/net/mac80211/tx.c
>>> +++ b/net/mac80211/tx.c
>>> @@ -1810,14 +1810,14 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
>>> if (!chanctx_conf)
>>> goto fail_rcu;
>>> band = chanctx_conf->def.chan->band;
>>> - if (sta)
>>> - break;
>>> - /* fall through */
>>> + if (!sta)
>>> + goto try_next;
>>> + break;
>>> case NL80211_IFTYPE_AP:
>>> - if (sdata->vif.type == NL80211_IFTYPE_AP)
>>> - chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
>>> + chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
>>> if (!chanctx_conf)
>>> goto fail_rcu;
>>> +try_next:
>>
>> I don't think that's better than the (fairly obvious) fall-through, and
>> has a pretty odd goto. Also, depending on the compiler, it still knows
>> the previous case label and doesn't warn.
>>
>
> Yeah, fall-through is obvious. But check 'A' again just near by "case A"
> seems a little strange, and some of compilers (or some of versions) are
> really not quit smart enough to know it is not a warning.
>
Sorry, the paragraph above may lead misunderstanding, I repeated again:
- fall-through is obvious (although I did not notice it, originally).
- Check 'A' again just near by "case A" seems a little strange.
- Some compilers aren't quit smart enough to know 'chanctx_conf' is OK.
Thanks.
> Hmm... for me, if the code (implementation) can express real logical
> work flow as much as directly and simply, the code (implementation) is
> clear enough (don't mind whether use 'goto' or not).
>
>
> And originally, at first, I am really not quite careful enough, and sent
> an incorrect patch after noticed the related compiler's warning. :-)
>
>
> Thanks.
>
--
Chen Gang
^ permalink raw reply
* Re: [net-next 3/7] openvswitch: Enable memory mapped Netlink i/o
From: Thomas Graf @ 2013-11-30 14:04 UTC (permalink / raw)
To: Florian Westphal
Cc: jesse, davem, dev, netdev, dborkman, ffusco, fleitner,
eric.dumazet, bhutchings
In-Reply-To: <20131130123549.GH2067@breakpoint.cc>
On 11/30/13 at 01:35pm, Florian Westphal wrote:
> Thomas Graf <tgraf@suug.ch> wrote:
> > 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
> [..]
> > After:
> > Result: OK: 24229690(c24127165+d102524) usec, 5000000 (9000byte,0frags)
> > 206358pps 14857Mb/sec (14857776000bps) errors: 0
>
> I'm curious. Is the 'old' value with skb_zerocopy() or without?
> Does ovs-vswitchd 'read-access' the entire packet or just e.g. the
> header?
>
> I ask because in netfilter nfqueue tests I could not see any difference
> between 'zerocopy' vs. mmap in the receive-path tests I made a while
> back.
The numbers quoted do not involve any zerocopy at all. It's a pure
original vs. mmap comparison. I expect the numbers to grow even more
once we get rid of the immediate ofpbuf copy in user space.
Zerocopy as implemened right now does not provide much gains on
top of mmap. We have no way yet to inject the skb frags into the
ring so we are forced to copy the data. The mapped skb is simply a
buffer with a tremendous tailroom to the zerocopy code.
Eventually we can find a way to make the skb frags shared and link
them to the ring buffer and avoid the copy. This would especially
make sense if we add GSO support to openvswitch user space as nfqueue
provides and avoid the segmentation before the upcall.
^ permalink raw reply
* Re: [PATCH] sctp: check the rto_min and rto_max
From: Neil Horman @ 2013-11-30 14:17 UTC (permalink / raw)
To: wangweidong; +Cc: Vlad Yasevich, davem, linux-sctp, netdev, dingtianhong
In-Reply-To: <5299BCB8.6070409@huawei.com>
On Sat, Nov 30, 2013 at 06:23:52PM +0800, wangweidong wrote:
> rto_min should be smaller than rto_max while rto_max should be larger
> than rto_min. so just add the check.
>
> Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
> ---
> net/sctp/sysctl.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/sctp/sysctl.c b/net/sctp/sysctl.c
> index 6b36561..7637e8e 100644
> --- a/net/sctp/sysctl.c
> +++ b/net/sctp/sysctl.c
> @@ -104,7 +104,7 @@ static struct ctl_table sctp_net_table[] = {
> .mode = 0644,
> .proc_handler = proc_dointvec_minmax,
> .extra1 = &one,
> - .extra2 = &timer_max
> + .extra2 = &init_net.sctp.rto_max
> },
> {
> .procname = "rto_max",
> @@ -112,7 +112,7 @@ static struct ctl_table sctp_net_table[] = {
> .maxlen = sizeof(unsigned int),
> .mode = 0644,
> .proc_handler = proc_dointvec_minmax,
> - .extra1 = &one,
> + .extra1 = &init_net.sctp.rto_min,
> .extra2 = &timer_max
> },
> {
> --
> 1.7.12
>
>
>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
^ permalink raw reply
* Re: [net v2 2/7] e1000: prevent oops when adapter is being closed and reset simultaneously
From: Sergei Shtylyov @ 2013-11-30 16:37 UTC (permalink / raw)
To: Jeff Kirsher, davem; +Cc: yzhu1, netdev, gospo, sassmann
In-Reply-To: <1385803221-3752-3-git-send-email-jeffrey.t.kirsher@intel.com>
Hello.
On 30-11-2013 13:20, Jeff Kirsher wrote:
> From: yzhu1 <yanjun.zhu@windriver.com>
> This change is based on a similar change made to e1000e support in
> commit bb9e44d0d0f4 ("e1000e: prevent oops when adapter is being closed
> and reset simultaneously"). The same issue has also been observed
> on the older e1000 cards.
> Here, we have increased the RESET_COUNT value to 50 because there are too
> many accesses to e1000 nic on stress tests to e1000 nic, it is not enough
> to set RESET_COUT 25. Experimentation has shown that it is enough to set
> RESET_COUNT 50.
> Signed-off-by: yzhu1 <yanjun.zhu@windriver.com>
Jeff, why are you accepting patches without real name in the sign-off line?
> Tested-by: Aaron Brown <aaron.f.brown@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
WBR, Sergei
^ permalink raw reply
* Re: [PATCH] ipv6: fix possible seqlock deadlock in ip6_finish_output2
From: Eric Dumazet @ 2013-11-30 16:45 UTC (permalink / raw)
To: Hannes Frederic Sowa
Cc: jongman.heo, David Miller, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20131129053944.GH24171@order.stressinduktion.org>
On Fri, 2013-11-29 at 06:39 +0100, Hannes Frederic Sowa wrote:
> IPv6 stats are 64 bits and thus are protected with a seqlock. By not
> disabling bottom-half we could deadlock here if we don't disable bh and
> a softirq reentrantly updates the same mib.
>
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> ---
Acked-by: Eric Dumazet <edumazet@google.com>
(Note that the seqlock is only for 32bit arches)
^ permalink raw reply
* Re: [PATCH] ipv6: ip6_pkt_prohibit(_out) should not depend on CONFIG_IPV6_MULTIPLE_TABLES
From: Hannes Frederic Sowa @ 2013-11-30 17:46 UTC (permalink / raw)
To: David Miller; +Cc: kamala, netdev, linux-kernel
In-Reply-To: <20131129.163930.203685593163996122.davem@davemloft.net>
On Fri, Nov 29, 2013 at 04:39:30PM -0500, David Miller wrote:
> From: Hannes Frederic Sowa <hannes@stressinduktion.org>
> Date: Thu, 28 Nov 2013 20:04:11 +0100
>
> > Since patch "IPv6: Fixed support for blackhole and prohibit routes" we
> > need ip6_pkt_prohibit(_out) available without CONFIG_IPV6_MULTIPLE_TABLES
> > defined.
> >
> > Cc: Kamala R <kamala@aristanetworks.com>
> > Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
>
> Please reference the SHA1 ID of the change as well as it's commit
> header text, thank you.
This patch is a follow-up patch for the first one in this thread which is not
committed, yet. Either Kamala can squash them into one or I'll repost it
later.
Greetings,
Hannes
^ permalink raw reply
* Re: [net v2 0/7][pull request] Intel Wired LAN Driver Updates
From: David Miller @ 2013-11-30 17:46 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, gospo, sassmann
In-Reply-To: <1385803221-3752-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Sat, 30 Nov 2013 01:20:14 -0800
> This series contains updates to igb, e1000 and ixgbe.
...
> git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net master
Pulled, but as mentioned by others please get real names from people
in their Signoffs in the future. Thanks.
^ permalink raw reply
* Re: [PATCH] ipv6: fix possible seqlock deadlock in ip6_finish_output2
From: David Miller @ 2013-11-30 17:49 UTC (permalink / raw)
To: eric.dumazet; +Cc: hannes, jongman.heo, netdev, linux-kernel
In-Reply-To: <1385829955.5352.40.camel@edumazet-glaptop2.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Sat, 30 Nov 2013 08:45:55 -0800
> On Fri, 2013-11-29 at 06:39 +0100, Hannes Frederic Sowa wrote:
>> IPv6 stats are 64 bits and thus are protected with a seqlock. By not
>> disabling bottom-half we could deadlock here if we don't disable bh and
>> a softirq reentrantly updates the same mib.
>>
>> Cc: Eric Dumazet <eric.dumazet@gmail.com>
>> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
>> ---
>
> Acked-by: Eric Dumazet <edumazet@google.com>
>
> (Note that the seqlock is only for 32bit arches)
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH net 0/3] net/hsr: Fix omissions
From: David Miller @ 2013-11-30 17:49 UTC (permalink / raw)
To: arvid.brodin
Cc: netdev, shemminger, joe, jboticario, balferreira, elias.molina
In-Reply-To: <52991680.1020707@alten.se>
From: Arvid Brodin <arvid.brodin@alten.se>
Date: Fri, 29 Nov 2013 23:34:40 +0100
> This is a patch series that fixes a few omissions in the original HSR patch.
>
> The HSR driver is completely new in v3.13-rc1, so the risk that someone would
> hit a regression should be very small. Also, the HSR driver is pretty useless
> without iproute support, which depends on patch 3 in the series. So I hope
> these are OK for net.
Series applied.
^ 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