* Re: [patch net-next RFC 1/2] ipv6 addrconf: extend ifa_flags to u32
From: Jiri Pirko @ 2013-11-11 15:12 UTC (permalink / raw)
To: David Laight
Cc: netdev, davem, kuznet, jmorris, yoshfuji, kaber, thaller, stephen,
hannes, vyasevich, dcbw
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B73FA@saturn3.aculab.com>
Mon, Nov 11, 2013 at 04:07:20PM CET, David.Laight@ACULAB.COM wrote:
>> --- a/include/net/if_inet6.h
>> +++ b/include/net/if_inet6.h
>> @@ -51,7 +51,7 @@ struct inet6_ifaddr {
>> int state;
>>
>> __u8 dad_probes;
>> - __u8 flags;
>> + __u32 flags;
>>
>> __u16 scope;
>
>That is adding a lot of padding.
Allright. Will reorder. Thanks
>
> David
>
>
>
^ permalink raw reply
* RE: TCP performance regression
From: Eric Dumazet @ 2013-11-11 15:29 UTC (permalink / raw)
To: David Laight; +Cc: Sujith Manoharan, netdev, Dave Taht
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B73F9@saturn3.aculab.com>
On Mon, 2013-11-11 at 15:05 +0000, David Laight wrote:
> Or, maybe:
> 5) call skb_orphan() (I think that is the correct function) when transmit
> packets are given to the hardware.
This is the worth possible solution, as it basically re-enables
bufferbloat again.
socket sk_wmem_queued should not be fooled, unless we have no other
choice.
^ permalink raw reply
* Re: [PATCH] phy: Add MOXA RTL8201CP PHY support
From: Grant Likely @ 2013-11-11 15:30 UTC (permalink / raw)
To: Florian Fainelli, Jonas Jensen
Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, netdev
In-Reply-To: <CAGVrzcZW=CLWPZgRqkqR1wNh+S7dfwRZAzn1Uq7Ji6K0=iGYtg@mail.gmail.com>
On Mon, 4 Nov 2013 09:51:24 -0800, Florian Fainelli <f.fainelli@gmail.com> wrote:
> 2013/11/4 Jonas Jensen <jonas.jensen@gmail.com>:
> > This is why I ended up setting PHY_IGNORE_INTERRUPT and the comment
> > about its effectiveness. Polling works but the extra reads on the bus
> > seem unnecessary.
> > Ideas how they can be eliminated are appreciated.
>
> As of today, the only way to work around it is not to use
> of_mdiobus_register() and use mdiobus_register() directly which will
> allow you to properly describe such a configuration.
of_mdiobus_register() can be changed. One option: If the caller knows
what it wants to happen in the case of no interrupt, then it can be
added as an argument to of_mdiobus_register(). No need to work around
this in an ugly way.
Is setting the irq to PHY_POLL something that should be done for the
entire bus, or only for a handful of PHYs?
g.
^ permalink raw reply
* Re: [PATCH v2] mac80211: add assoc beacon timeout logic
From: Johannes Berg @ 2013-11-11 15:43 UTC (permalink / raw)
To: Felipe Contreras
Cc: linux-wireless Mailing List, netdev, John W. Linville,
David S. Miller
In-Reply-To: <CAMP44s3TFSgWuR8gVoFBznNP+t9mz+to===9i1PAnra4ogKu9w@mail.gmail.com>
On Mon, 2013-11-11 at 04:59 -0600, Felipe Contreras wrote:
> Well the AP is sending beacons, but they seem to be corrupted,
> although the corruption often seems to happen in a place that is not
> so important.
Indeed - the beacon you sent to me in private is damaged somewhere
towards the end of the frame. Are we actually receiving it but ignoring
it because it doesn't have the data we need? The firmware still
shouldn't be filtering anything since it doesn't really look at the
beacon information (or maybe it filters based on the DS IE? I'm not
entirely sure)
> No other device at this house seems to have a problem with that, even
> the Intel driver in Windows doesn't have a problem, it's just the
> driver in Linux.
The windows driver has some slightly different behaviour - it pretends
to be associated towards the firmware before that's actually true, iirc.
> However, if I apply this patch, I don't notice any issue, it
> associates and works fine. Maybe there's some subtle issues with
> features I don't personally use, or perhaps there's the occasional
> disconnection (although that could be due to something else), but
> that's light years away from not associating at all.
>
> I'd say between a) some features not working and b) nothing working at
> all, a) is preferred.
>
> If you think it's better that nothing works at all, then wouldn't it
> make sense to time out and return an error? Currently we just keep
> trying to associate *forever*.
That's wpa_supplicant/userspace behaviour. The kernel will just drop the
connection.
> > If the AP is sending beacons but the device isn't receiving them, then
> > it's a driver bug and mac80211 shouldn't work around it.
>
> I agree, but I can't seem to convince Intel guys of that. The firmware
> is dropping the corrupt beacon frames (although not always), so
> there's nothing the driver can do afterwards.
You realize I work for the same team in Intel as well? :)
> But even if there were not beacons at all (corrupt or otherwise), I
> still think waiting *forever* in a loop is not ideal, a) is preferred;
> not having all the features, but still somehow work (from my point of
> view it's more than somewhat).
This isn't really true like I said above - the kernel can only drop the
association, if userspace *insists* then it will try again and again.
I'd much rather try to get to the bottom of this. Maybe the firmware is
dropping the beacon because the DS IE is broken? Or are we receiving it
but ignoring it because it's broken?
Unfortunately, there's only so much we can do to work around broken APs.
johannes
^ permalink raw reply
* RE: TCP performance regression
From: David Laight @ 2013-11-11 15:43 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Sujith Manoharan, netdev, Dave Taht
In-Reply-To: <1384183799.16391.39.camel@edumazet-glaptop2.roam.corp.google.com>
> > Or, maybe:
> > 5) call skb_orphan() (I think that is the correct function) when transmit
> > packets are given to the hardware.
>
> This is the worth possible solution, as it basically re-enables
^^^^^ worst ?
> bufferbloat again.
It should be ok if the mac driver only gives the hardware a small
number of bytes/packets - or one appropriate for the link speed.
David
^ permalink raw reply
* [PATCH 2/3 net-next] openvswitch: Allow user space to announce ability to accept unaligned Netlink messages
From: Thomas Graf @ 2013-11-11 15:47 UTC (permalink / raw)
To: jesse, davem; +Cc: dev, netdev, eric.dumazet, dborkman, bhutchings
In-Reply-To: <cover.1384184208.git.tgraf@suug.ch>
Signed-off-by: Thomas Graf <tgraf@suug.ch>
---
include/uapi/linux/openvswitch.h | 4 ++++
net/openvswitch/datapath.c | 4 ++++
net/openvswitch/datapath.h | 2 ++
3 files changed, 10 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 1408adc..0a50574 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -1061,6 +1061,7 @@ static 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 = {
@@ -1217,6 +1218,9 @@ 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]);
+ if (a[OVS_DP_ATTR_USER_FEATURES])
+ dp->user_features = nla_get_u32(a[OVS_DP_ATTR_USER_FEATURES]);
+
vport = new_vport(&parms);
if (IS_ERR(vport)) {
err = PTR_ERR(vport);
diff --git a/net/openvswitch/datapath.h b/net/openvswitch/datapath.h
index d3d14a58..ecdf44c 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 1/3 net-next] net: Export skb_zerocopy() to zerocopy from one skb to another
From: Thomas Graf @ 2013-11-11 15:47 UTC (permalink / raw)
To: jesse, davem; +Cc: dev, netdev, eric.dumazet, dborkman, bhutchings
In-Reply-To: <cover.1384184208.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>
---
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 036ec7d..9e6a293 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2372,6 +2372,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 8c5197f..c5cefbe 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -2125,6 +2125,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 0/3 v5] Open vSwitch zerocopy upcall
From: Thomas Graf @ 2013-11-11 15:47 UTC (permalink / raw)
To: jesse, davem; +Cc: dev, netdev, eric.dumazet, dborkman, bhutchings
Respin of the zerocopy patches for the openvswitch upcall.
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 (3):
net: Export skb_zerocopy() to zerocopy from one skb to another
openvswitch: Allow user space to announce ability to accept unaligned
Netlink messages
openvswitch: Use skb_zerocopy() for upcall
include/linux/skbuff.h | 3 ++
include/uapi/linux/openvswitch.h | 4 ++
net/core/skbuff.c | 85 ++++++++++++++++++++++++++++++++++++
net/netfilter/nfnetlink_queue_core.c | 59 ++-----------------------
net/openvswitch/datapath.c | 58 ++++++++++++++++--------
net/openvswitch/datapath.h | 2 +
6 files changed, 139 insertions(+), 72 deletions(-)
--
1.8.3.1
^ permalink raw reply
* [PATCH 3/3 net-next] openvswitch: Use skb_zerocopy() for upcall
From: Thomas Graf @ 2013-11-11 15:47 UTC (permalink / raw)
To: jesse, davem; +Cc: dev, netdev, eric.dumazet, dborkman, bhutchings
In-Reply-To: <cover.1384184208.git.tgraf@suug.ch>
Use of skb_zerocopy() avoids the expensive call to memcpy() when
copying the packet data into the Netlink skb. Completes checksum
through skb_checksum_help() if needed.
Cost of memcpy 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>
---
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 0a50574..8dc2496 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,14 +395,15 @@ 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;
struct sk_buff *nskb = NULL;
struct sk_buff *user_skb; /* to be queued to userspace */
struct nlattr *nla;
+ unsigned int hlen;
int err;
if (vlan_tx_tag_present(skb)) {
@@ -422,7 +424,21 @@ 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);
+ /* 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;
+
+ user_skb = genlmsg_new(upcall_msg_size(skb, upcall_info->userdata, hlen), GFP_ATOMIC);
if (!user_skb) {
err = -ENOMEM;
goto out;
@@ -441,13 +457,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
* RE: TCP performance regression
From: Eric Dumazet @ 2013-11-11 16:17 UTC (permalink / raw)
To: David Laight; +Cc: Sujith Manoharan, netdev, Dave Taht
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B73FB@saturn3.aculab.com>
On Mon, 2013-11-11 at 15:43 +0000, David Laight wrote:
> > > Or, maybe:
> > > 5) call skb_orphan() (I think that is the correct function) when transmit
> > > packets are given to the hardware.
> >
> > This is the worth possible solution, as it basically re-enables
> ^^^^^ worst ?
> > bufferbloat again.
>
> It should be ok if the mac driver only gives the hardware a small
> number of bytes/packets - or one appropriate for the link speed.
There is some confusion here.
mvneta has a TX ring buffer, which can hold up to 532 TX descriptors.
If this driver used skb_orphan(), a single TCP flow could use the whole
TX ring.
TCP Small Queue would only limit the number of skbs on Qdisc.
Try then to send a ping message, it will have to wait a lot.
^ permalink raw reply
* Re: TCP performance regression
From: Sujith Manoharan @ 2013-11-11 16:13 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Felix Fietkau, netdev, Dave Taht
In-Reply-To: <1384180069.16391.32.camel@edumazet-glaptop2.roam.corp.google.com>
Eric Dumazet wrote:
> We have many choices.
>
> 1) Add back a minimum of ~128 K of outstanding bytes per TCP session,
> so that buggy drivers can sustain 'line rate'.
>
> Note that with 100 concurrent TCP streams, total amount of bytes
> queued on the NIC is 12 MB.
> And pfifo_fast qdisc will drop packets anyway.
>
> Thats what we call 'BufferBloat'
>
> 2) Try lower values like 64K. Still bufferbloat.
>
> 3) Fix buggy drivers, using a proper logic, or shorter timers (mvneta
> case for example)
>
> 4) Add a new netdev attribute, so that well behaving NIC drivers do not
> have to artificially force TCP stack to queue too many bytes in
> Qdisc/NIC queues.
I think the quirks of 802.11 aggregation should be taken into account.
I am adding Felix to this thread, who would have more to say on latency/bufferbloat
with wireless drivers.
Sujith
^ permalink raw reply
* Re: [PATCH v2] mac80211: add assoc beacon timeout logic
From: Felipe Contreras @ 2013-11-11 16:23 UTC (permalink / raw)
To: Johannes Berg
Cc: linux-wireless Mailing List, netdev, John W. Linville,
David S. Miller
In-Reply-To: <1384184624.14334.31.camel-8Nb76shvtaUJvtFkdXX2HixXY32XiHfO@public.gmane.org>
On Mon, Nov 11, 2013 at 9:43 AM, Johannes Berg
<johannes-cdvu00un1VgdHxzADdlk8Q@public.gmane.org> wrote:
> On Mon, 2013-11-11 at 04:59 -0600, Felipe Contreras wrote:
>
>> Well the AP is sending beacons, but they seem to be corrupted,
>> although the corruption often seems to happen in a place that is not
>> so important.
>
> Indeed - the beacon you sent to me in private is damaged somewhere
> towards the end of the frame. Are we actually receiving it but ignoring
> it because it doesn't have the data we need?
The driver is not receiving it at all. I already debugged this:
http://article.gmane.org/gmane.linux.kernel.wireless.general/115429
However, I noticed that once in a very long time, sometimes it does
receive the corrupted frame and the association continues, and the
driver code detects it's a corrupted beacon frame.
> The firmware still
> shouldn't be filtering anything since it doesn't really look at the
> beacon information (or maybe it filters based on the DS IE? I'm not
> entirely sure)
That's what I thought, but I don't see it at all (only in monitor
mode, and in ad-hoc).
>> However, if I apply this patch, I don't notice any issue, it
>> associates and works fine. Maybe there's some subtle issues with
>> features I don't personally use, or perhaps there's the occasional
>> disconnection (although that could be due to something else), but
>> that's light years away from not associating at all.
>>
>> I'd say between a) some features not working and b) nothing working at
>> all, a) is preferred.
>>
>> If you think it's better that nothing works at all, then wouldn't it
>> make sense to time out and return an error? Currently we just keep
>> trying to associate *forever*.
>
> That's wpa_supplicant/userspace behaviour. The kernel will just drop the
> connection.
Nope, it keeps trying forever.
Oct 13 14:33:15 nysa kernel: wlan0: authenticate with e0:1d:3b:46:82:a0
Oct 13 14:33:15 nysa kernel: wlan0: send auth to e0:1d:3b:46:82:a0 (try 1/3)
Oct 13 14:33:15 nysa kernel: wlan0: authenticated
Oct 13 14:33:15 nysa kernel: wlan0: waiting for beacon from e0:1d:3b:46:82:a0
Oct 13 14:33:18 nysa kernel: wlan0: authenticate with e0:1d:3b:46:82:a0
Oct 13 14:33:18 nysa kernel: wlan0: send auth to e0:1d:3b:46:82:a0 (try 1/3)
Oct 13 14:33:18 nysa kernel: wlan0: authenticated
Oct 13 14:33:18 nysa kernel: wlan0: waiting for beacon from e0:1d:3b:46:82:a0
Oct 13 14:33:22 nysa kernel: wlan0: authenticate with e0:1d:3b:46:82:a0
Oct 13 14:33:22 nysa kernel: wlan0: send auth to e0:1d:3b:46:82:a0 (try 1/3)
Oct 13 14:33:22 nysa kernel: wlan0: authenticated
Oct 13 14:33:22 nysa kernel: wlan0: waiting for beacon from e0:1d:3b:46:82:a0
...
>> > If the AP is sending beacons but the device isn't receiving them, then
>> > it's a driver bug and mac80211 shouldn't work around it.
>>
>> I agree, but I can't seem to convince Intel guys of that. The firmware
>> is dropping the corrupt beacon frames (although not always), so
>> there's nothing the driver can do afterwards.
>
> You realize I work for the same team in Intel as well? :)
Now I do.
>> But even if there were not beacons at all (corrupt or otherwise), I
>> still think waiting *forever* in a loop is not ideal, a) is preferred;
>> not having all the features, but still somehow work (from my point of
>> view it's more than somewhat).
>
> This isn't really true like I said above - the kernel can only drop the
> association, if userspace *insists* then it will try again and again.
But it's not doing this:
ieee80211_destroy_assoc_data(sdata, false);
cfg80211_assoc_timeout(sdata->dev, bss);
Which is what causes the association to stop for me.
So where exactly in the code is the association being "dropped"?
> I'd much rather try to get to the bottom of this. Maybe the firmware is
> dropping the beacon because the DS IE is broken? Or are we receiving it
> but ignoring it because it's broken?
It's not the latter.
I would rather fix the problem at the two levels, so even if the
firmware passes the corrupt frames correctly, the driver would still
somewhat work when there's no beacon frames at all.
> Unfortunately, there's only so much we can do to work around broken APs.
Indeed, but 'so much' for this AP is really nothing, while with my
patch it's quite a lot.
--
Felipe Contreras
--
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 iproute2] ipaddress: add a black line for each device to make the output more readable
From: Jiri Benc @ 2013-11-11 16:31 UTC (permalink / raw)
To: Hangbin Liu; +Cc: network dev
In-Reply-To: <1384182210-8901-1-git-send-email-liuhangbin@gmail.com>
On Mon, 11 Nov 2013 23:03:30 +0800, Hangbin Liu wrote:
> When we have multi links, the output huddled together and make it hard to read.
> Let's use the old ifconfig output style.
No. You can't break others' scripts like that.
bash$ function ip { /sbin/ip "$@" | sed '2,$s/^[0-9]*:/\n&/' ; }
^^^ Here, fixed that for you.
Jiri
--
Jiri Benc
^ permalink raw reply
* RE: TCP performance regression
From: David Laight @ 2013-11-11 16:35 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Sujith Manoharan, netdev, Dave Taht
In-Reply-To: <1384186622.16391.44.camel@edumazet-glaptop2.roam.corp.google.com>
> > It should be ok if the mac driver only gives the hardware a small
> > number of bytes/packets - or one appropriate for the link speed.
>
> There is some confusion here.
>
> mvneta has a TX ring buffer, which can hold up to 532 TX descriptors.
>
> If this driver used skb_orphan(), a single TCP flow could use the whole
> TX ring.
>
> TCP Small Queue would only limit the number of skbs on Qdisc.
>
> Try then to send a ping message, it will have to wait a lot.
532 is a ridiculously large number especially for a slow interface.
At a guess you don't want more than 10-20ms of data in the tx ring.
You might need extra descriptors for badly fragmented packets.
David
^ permalink raw reply
* Re: TCP performance regression
From: Felix Fietkau @ 2013-11-11 16:38 UTC (permalink / raw)
To: Sujith Manoharan, Eric Dumazet; +Cc: netdev, Dave Taht
In-Reply-To: <21121.575.539384.948990@gargle.gargle.HOWL>
On 2013-11-11 17:13, Sujith Manoharan wrote:
> Eric Dumazet wrote:
>> We have many choices.
>>
>> 1) Add back a minimum of ~128 K of outstanding bytes per TCP session,
>> so that buggy drivers can sustain 'line rate'.
>>
>> Note that with 100 concurrent TCP streams, total amount of bytes
>> queued on the NIC is 12 MB.
>> And pfifo_fast qdisc will drop packets anyway.
>>
>> Thats what we call 'BufferBloat'
>>
>> 2) Try lower values like 64K. Still bufferbloat.
>>
>> 3) Fix buggy drivers, using a proper logic, or shorter timers (mvneta
>> case for example)
>>
>> 4) Add a new netdev attribute, so that well behaving NIC drivers do not
>> have to artificially force TCP stack to queue too many bytes in
>> Qdisc/NIC queues.
>
> I think the quirks of 802.11 aggregation should be taken into account.
> I am adding Felix to this thread, who would have more to say on latency/bufferbloat
> with wireless drivers.
I don't think this issue is about something as simple as timer handling
for tx completion (or even broken/buggy drivers).
There's simply no way to make 802.11 aggregation work well and have
similar tx completion latency characteristics as Ethernet devices.
802.11 aggregation reduces the per-packet airtime overhead by combining
multiple packets into one transmission (saving a lot of time getting a
tx opportunity, transmitting the PHY header, etc.), which makes the
'line rate' heavily depend on the amount of buffering.
Aggregating multiple packets into one transmission also causes extra
packet loss, which is compensated by retransmission and reordering, thus
introducing additional latency.
I don't think that TSQ can do a decent job of mitigating bufferbloat on
802.11n devices without a significant performance hit, so adding a new
netdev attribute might be a good idea.
- Felix
^ permalink raw reply
* Re: [PATCH v2] mac80211: add assoc beacon timeout logic
From: Johannes Berg @ 2013-11-11 16:41 UTC (permalink / raw)
To: Felipe Contreras
Cc: linux-wireless Mailing List, netdev, John W. Linville,
David S. Miller
In-Reply-To: <CAMP44s2cVVe4-yW214B9hJu7n6M1onZpZaPCj5N0Fi1JWDKKsw@mail.gmail.com>
On Mon, 2013-11-11 at 10:23 -0600, Felipe Contreras wrote:
> The driver is not receiving it at all. I already debugged this:
>
> http://article.gmane.org/gmane.linux.kernel.wireless.general/115429
Hmm, ok. I pretty much didn't read that thread since some others were
jumping in.
> However, I noticed that once in a very long time, sometimes it does
> receive the corrupted frame and the association continues, and the
> driver code detects it's a corrupted beacon frame.
So how does it treat the corruption?
> > The firmware still
> > shouldn't be filtering anything since it doesn't really look at the
> > beacon information (or maybe it filters based on the DS IE? I'm not
> > entirely sure)
>
> That's what I thought, but I don't see it at all (only in monitor
> mode, and in ad-hoc).
Yes, that part is odd - that's really the root cause.
I didn't quickly find in the threads what device and firmware you were
using, mind identifying it (again)?
> Nope, it keeps trying forever.
>
> Oct 13 14:33:15 nysa kernel: wlan0: authenticate with e0:1d:3b:46:82:a0
> Oct 13 14:33:15 nysa kernel: wlan0: send auth to e0:1d:3b:46:82:a0 (try 1/3)
> Oct 13 14:33:15 nysa kernel: wlan0: authenticated
> Oct 13 14:33:15 nysa kernel: wlan0: waiting for beacon from e0:1d:3b:46:82:a0
> Oct 13 14:33:18 nysa kernel: wlan0: authenticate with e0:1d:3b:46:82:a0
> Oct 13 14:33:18 nysa kernel: wlan0: send auth to e0:1d:3b:46:82:a0 (try 1/3)
> Oct 13 14:33:18 nysa kernel: wlan0: authenticated
> Oct 13 14:33:18 nysa kernel: wlan0: waiting for beacon from e0:1d:3b:46:82:a0
> Oct 13 14:33:22 nysa kernel: wlan0: authenticate with e0:1d:3b:46:82:a0
> Oct 13 14:33:22 nysa kernel: wlan0: send auth to e0:1d:3b:46:82:a0 (try 1/3)
> Oct 13 14:33:22 nysa kernel: wlan0: authenticated
> Oct 13 14:33:22 nysa kernel: wlan0: waiting for beacon from e0:1d:3b:46:82:a0
> ...
I see the same behaviour - but it's the supplicant's doing, it is indeed
getting the event that the AP connection failed (timed out):
wlan0: Event ASSOC_TIMED_OUT (15) received
> > This isn't really true like I said above - the kernel can only drop the
> > association, if userspace *insists* then it will try again and again.
>
> But it's not doing this:
>
> ieee80211_destroy_assoc_data(sdata, false);
> cfg80211_assoc_timeout(sdata->dev, bss);
>
> Which is what causes the association to stop for me.
>
> So where exactly in the code is the association being "dropped"?
This does get called in my setup.
> I would rather fix the problem at the two levels, so even if the
> firmware passes the corrupt frames correctly, the driver would still
> somewhat work when there's no beacon frames at all.
Like I said before - trying to work with an AP without beacons at all is
really bad, we shouldn't be doing it. We might not properly react to
radar events, and other things, for example.
johannes
^ permalink raw reply
* Re: TCP performance regression
From: Eric Dumazet @ 2013-11-11 16:44 UTC (permalink / raw)
To: Sujith Manoharan; +Cc: Arnaud Ebalard, netdev, Dave Taht, Thomas Petazzoni
In-Reply-To: <1384180787.16391.35.camel@edumazet-glaptop2.roam.corp.google.com>
On Mon, 2013-11-11 at 06:39 -0800, Eric Dumazet wrote:
> How following patch helps mvneta performance on current net-next tree
> for a single TCP (sending) flow ?
>
v2 (more chance to even compile ;)
diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index 7d99e695a110..e8211277f15d 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -172,12 +172,12 @@
/* Various constants */
/* Coalescing */
-#define MVNETA_TXDONE_COAL_PKTS 16
+#define MVNETA_TXDONE_COAL_PKTS 1
#define MVNETA_RX_COAL_PKTS 32
#define MVNETA_RX_COAL_USEC 100
/* Timer */
-#define MVNETA_TX_DONE_TIMER_PERIOD 10
+#define MVNETA_TX_DONE_TIMER_PERIOD 1
/* Napi polling weight */
#define MVNETA_RX_POLL_WEIGHT 64
^ permalink raw reply related
* Re: [PATCH v2] mac80211: add assoc beacon timeout logic
From: Felipe Contreras @ 2013-11-11 16:53 UTC (permalink / raw)
To: Johannes Berg
Cc: linux-wireless Mailing List, netdev, John W. Linville,
David S. Miller
In-Reply-To: <1384188067.14334.45.camel@jlt4.sipsolutions.net>
On Mon, Nov 11, 2013 at 10:41 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Mon, 2013-11-11 at 10:23 -0600, Felipe Contreras wrote:
>> However, I noticed that once in a very long time, sometimes it does
>> receive the corrupted frame and the association continues, and the
>> driver code detects it's a corrupted beacon frame.
>
> So how does it treat the corruption?
wlan0: associating with AP with corrupt beacon
>> > The firmware still
>> > shouldn't be filtering anything since it doesn't really look at the
>> > beacon information (or maybe it filters based on the DS IE? I'm not
>> > entirely sure)
>>
>> That's what I thought, but I don't see it at all (only in monitor
>> mode, and in ad-hoc).
>
> Yes, that part is odd - that's really the root cause.
>
> I didn't quickly find in the threads what device and firmware you were
> using, mind identifying it (again)?
Intel Corporation Centrino Advanced-N 6235 (rev 24)
iwlwifi 0000:02:00.0: loaded firmware version 18.168.6.1 op_mode iwldvm
>> Nope, it keeps trying forever.
>>
>> Oct 13 14:33:15 nysa kernel: wlan0: authenticate with e0:1d:3b:46:82:a0
>> Oct 13 14:33:15 nysa kernel: wlan0: send auth to e0:1d:3b:46:82:a0 (try 1/3)
>> Oct 13 14:33:15 nysa kernel: wlan0: authenticated
>> Oct 13 14:33:15 nysa kernel: wlan0: waiting for beacon from e0:1d:3b:46:82:a0
>> Oct 13 14:33:18 nysa kernel: wlan0: authenticate with e0:1d:3b:46:82:a0
>> Oct 13 14:33:18 nysa kernel: wlan0: send auth to e0:1d:3b:46:82:a0 (try 1/3)
>> Oct 13 14:33:18 nysa kernel: wlan0: authenticated
>> Oct 13 14:33:18 nysa kernel: wlan0: waiting for beacon from e0:1d:3b:46:82:a0
>> Oct 13 14:33:22 nysa kernel: wlan0: authenticate with e0:1d:3b:46:82:a0
>> Oct 13 14:33:22 nysa kernel: wlan0: send auth to e0:1d:3b:46:82:a0 (try 1/3)
>> Oct 13 14:33:22 nysa kernel: wlan0: authenticated
>> Oct 13 14:33:22 nysa kernel: wlan0: waiting for beacon from e0:1d:3b:46:82:a0
>> ...
>
> I see the same behaviour - but it's the supplicant's doing, it is indeed
> getting the event that the AP connection failed (timed out):
>
> wlan0: Event ASSOC_TIMED_OUT (15) received
Not in my setup.
>> > This isn't really true like I said above - the kernel can only drop the
>> > association, if userspace *insists* then it will try again and again.
>>
>> But it's not doing this:
>>
>> ieee80211_destroy_assoc_data(sdata, false);
>> cfg80211_assoc_timeout(sdata->dev, bss);
>>
>> Which is what causes the association to stop for me.
>>
>> So where exactly in the code is the association being "dropped"?
>
> This does get called in my setup.
Yes, because your setup is receiving beacons.
Check the code:
if ((ifmgd->assoc_data->need_beacon && !ifmgd->have_beacon) ||
ieee80211_do_assoc(sdata)) {
struct cfg80211_bss *bss = ifmgd->assoc_data->bss;
ieee80211_destroy_assoc_data(sdata, false);
cfg80211_assoc_timeout(sdata->dev, bss);
}
If there's no beacon, cfg80211_assoc_timeout() is not called.
I'm sure if you don't call ieee80211_rx_mgmt_beacon() at all you will
see the same behavior I see.
>> I would rather fix the problem at the two levels, so even if the
>> firmware passes the corrupt frames correctly, the driver would still
>> somewhat work when there's no beacon frames at all.
>
> Like I said before - trying to work with an AP without beacons at all is
> really bad, we shouldn't be doing it.
Why not? For all intents and purposes my system is not receiving any
beacons, and I don't see any problems.
What would you prefer? That nothing works at all?
> We might not properly react to
> radar events, and other things, for example.
So? I don't know what that means, but it can't be worst than not being
able to connect to the Internet whatsoever at all.
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH v2] mac80211: add assoc beacon timeout logic
From: Felipe Contreras @ 2013-11-11 16:56 UTC (permalink / raw)
To: Johannes Berg
Cc: linux-wireless Mailing List, netdev, John W. Linville,
David S. Miller
In-Reply-To: <CAMP44s3nkj2wZYCPve7EhyeD7sEFmpUN=-h015dOP68PcNSTug@mail.gmail.com>
On Mon, Nov 11, 2013 at 10:53 AM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> On Mon, Nov 11, 2013 at 10:41 AM, Johannes Berg
> <johannes@sipsolutions.net> wrote:
>> On Mon, 2013-11-11 at 10:23 -0600, Felipe Contreras wrote:
>>> > This isn't really true like I said above - the kernel can only drop the
>>> > association, if userspace *insists* then it will try again and again.
>>>
>>> But it's not doing this:
>>>
>>> ieee80211_destroy_assoc_data(sdata, false);
>>> cfg80211_assoc_timeout(sdata->dev, bss);
>>>
>>> Which is what causes the association to stop for me.
>>>
>>> So where exactly in the code is the association being "dropped"?
>>
>> This does get called in my setup.
>
> Yes, because your setup is receiving beacons.
>
> Check the code:
>
> if ((ifmgd->assoc_data->need_beacon && !ifmgd->have_beacon) ||
> ieee80211_do_assoc(sdata)) {
> struct cfg80211_bss *bss = ifmgd->assoc_data->bss;
>
> ieee80211_destroy_assoc_data(sdata, false);
> cfg80211_assoc_timeout(sdata->dev, bss);
> }
>
> If there's no beacon, cfg80211_assoc_timeout() is not called.
>
> I'm sure if you don't call ieee80211_rx_mgmt_beacon() at all you will
> see the same behavior I see.
My bad, actually the code that is not being called is:
cfg80211_unlink_bss(local->hw.wiphy, assoc_data->bss);
In ieee80211_do_assoc().
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH v2] mac80211: add assoc beacon timeout logic
From: Johannes Berg @ 2013-11-11 17:00 UTC (permalink / raw)
To: Felipe Contreras
Cc: linux-wireless Mailing List, netdev, John W. Linville,
David S. Miller
In-Reply-To: <CAMP44s3nkj2wZYCPve7EhyeD7sEFmpUN=-h015dOP68PcNSTug@mail.gmail.com>
On Mon, 2013-11-11 at 10:53 -0600, Felipe Contreras wrote:
> > I see the same behaviour - but it's the supplicant's doing, it is indeed
> > getting the event that the AP connection failed (timed out):
> >
> > wlan0: Event ASSOC_TIMED_OUT (15) received
>
> Not in my setup.
Well, dunno then. Different kernel versions? This clearly happens for
me.
> >> > This isn't really true like I said above - the kernel can only drop the
> >> > association, if userspace *insists* then it will try again and again.
> >>
> >> But it's not doing this:
> >>
> >> ieee80211_destroy_assoc_data(sdata, false);
> >> cfg80211_assoc_timeout(sdata->dev, bss);
> >>
> >> Which is what causes the association to stop for me.
> >>
> >> So where exactly in the code is the association being "dropped"?
> >
> > This does get called in my setup.
>
> Yes, because your setup is receiving beacons.
No ... I tested on hwsim, making it ask for dtim-before-assoc, and
short-circuiting the beacon-TX routing. It can't have been seeing
beacons.
> Check the code:
>
> if ((ifmgd->assoc_data->need_beacon && !ifmgd->have_beacon) ||
> ieee80211_do_assoc(sdata)) {
> struct cfg80211_bss *bss = ifmgd->assoc_data->bss;
>
> ieee80211_destroy_assoc_data(sdata, false);
> cfg80211_assoc_timeout(sdata->dev, bss);
> }
>
> If there's no beacon, cfg80211_assoc_timeout() is not called.
Yes it is.
"need_beacon && !have_beacon:
means - I wanted the beacon but didn't get it at the timeout.
> I'm sure if you don't call ieee80211_rx_mgmt_beacon() at all you will
> see the same behavior I see.
I'm sure I won't :)
> > Like I said before - trying to work with an AP without beacons at all is
> > really bad, we shouldn't be doing it.
>
> Why not? For all intents and purposes my system is not receiving any
> beacons, and I don't see any problems.
The not receiving part is a bug. I think you're probably receiving
beacons once associated though?
> What would you prefer? That nothing works at all?
Yes, that'd be much safer.
> > We might not properly react to
> > radar events, and other things, for example.
>
> So? I don't know what that means, but it can't be worst than not being
> able to connect to the Internet whatsoever at all.
It can make you break the law.
johannes
^ permalink raw reply
* Re: [PATCH v2] mac80211: add assoc beacon timeout logic
From: Johannes Berg @ 2013-11-11 17:01 UTC (permalink / raw)
To: Felipe Contreras
Cc: linux-wireless Mailing List, netdev, John W. Linville,
David S. Miller
In-Reply-To: <CAMP44s0Lsd2qMf3a-Vx4=HTd_UxzLVC6q1bAGPr+t6anubNknA@mail.gmail.com>
On Mon, 2013-11-11 at 10:56 -0600, Felipe Contreras wrote:
> On Mon, Nov 11, 2013 at 10:53 AM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
> > On Mon, Nov 11, 2013 at 10:41 AM, Johannes Berg
> > <johannes@sipsolutions.net> wrote:
> >> On Mon, 2013-11-11 at 10:23 -0600, Felipe Contreras wrote:
>
> >>> > This isn't really true like I said above - the kernel can only drop the
> >>> > association, if userspace *insists* then it will try again and again.
> >>>
> >>> But it's not doing this:
> >>>
> >>> ieee80211_destroy_assoc_data(sdata, false);
> >>> cfg80211_assoc_timeout(sdata->dev, bss);
> >>>
> >>> Which is what causes the association to stop for me.
> >>>
> >>> So where exactly in the code is the association being "dropped"?
> >>
> >> This does get called in my setup.
> >
> > Yes, because your setup is receiving beacons.
> >
> > Check the code:
> >
> > if ((ifmgd->assoc_data->need_beacon && !ifmgd->have_beacon) ||
> > ieee80211_do_assoc(sdata)) {
> > struct cfg80211_bss *bss = ifmgd->assoc_data->bss;
> >
> > ieee80211_destroy_assoc_data(sdata, false);
> > cfg80211_assoc_timeout(sdata->dev, bss);
> > }
> >
> > If there's no beacon, cfg80211_assoc_timeout() is not called.
> >
> > I'm sure if you don't call ieee80211_rx_mgmt_beacon() at all you will
> > see the same behavior I see.
>
> My bad, actually the code that is not being called is:
>
> cfg80211_unlink_bss(local->hw.wiphy, assoc_data->bss);
>
> In ieee80211_do_assoc().
That's not really interesting though, it just deletes the scan entry. If
it was deleted, then the supplicant would just scan again and probably
retry the connection.
johannes
^ permalink raw reply
* RE: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.
From: David Laight @ 2013-11-11 17:12 UTC (permalink / raw)
To: Alan Stern
Cc: Sarah Sharp, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <Pine.LNX.4.44L0.1311081303460.1162-100000-IYeN2dnnYyZXsRXLowluHWD2FQJk+8+b@public.gmane.org>
> From: Alan Stern [mailto:stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org]
> On Fri, 8 Nov 2013, David Laight wrote:
>
...
> GET_MAX_PACKET always returns MaxPacketSize, and for USB-3 bulk
> endpoints, MaxPacketSize is always 1024. MaxBurstSize can be anything
> from 1 to 16.
I've just read something that explained about bursts.
> > > According to my version of the spec (Rev 1.0, dated 5/21/2010), if a TD
> > > is larger than the MBP and its length isn't a multiple of the MBP, then
> > > the last MBP boundary in the TD must also be a TRB boundary. This
> > > follows from two statements in section 4.11.7.1:
> > >
> > > If the TD Transfer Size is an even multiple of the MBP then all
> > > TD Fragments shall define exact multiples of MBP data bytes.
> > > If not, then only the last TD Fragment shall define less than
> > > MBP data (or the Residue) bytes.
> >
> > No, that doesn't stop there being only 1 TD fragment.
> > (I think it means exact multiple of the MBP.)
>
> Suppose, for example, the MBP is 1024. If you have a TD with length
> 1500, and if it had only one fragment, the last (and only) fragment's
> length would not less than the MBP and it would not be an exact
> multiple of the MBP.
That doesn't matter - eg example 2 in figure 25
> I agree that the text is not as clear as it should be.
Reading it all again makes me think that a LINK trb is only
allowed on the burst boundary (which might be 16k bytes).
The only real way to implement that is to ensure that TD never
contain LINK TRB.
There are other things that can be done at a TD fragment boundary
(or only once per TD fragment) - but the code doesn't attempt any
of them.
The restriction might be there to simplify the hardware to
retransmit as TRB burst.
My USB3 ethernet card (ax88179_178a driver) is running a lot more
reliably with these changes.
David
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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: [net-next PATCH v4 0/2] l2 hardware accelerated macvlans
From: John Fastabend @ 2013-11-11 17:16 UTC (permalink / raw)
To: vyasevic
Cc: nhorman, davem, alexander.h.duyck, andy, netdev,
jeffrey.t.kirsher, vyasevich
In-Reply-To: <527C662B.4080300@redhat.com>
On 11/7/2013 8:18 PM, Vlad Yasevich wrote:
[...]
>> If folks find this series acceptable there are a few
>> items we can work on next. First broadcast and multicast
>> will use the hardware even for local traffic with this
>> series. It would be best (I think) to use the software
>> path for macvlan to macvlan traffic and save the PCIe
>> bus. This depends on how much you value CPU time vs
>> PCIE bandwidth. This will need another patch series
>> to flush out.
>>
>
> John
>
> So, I've been looking at these patches and the more I
> look the more I wonder how much benefit there is in
> sending unicast macvlan<->macvlan traffic through the hw.
> It looks like any bulk transfers would have to undergo
> tso segmentaion and GRO, where as this can be completely
> bypassed right now with software.
>
> Did you run any numbers?
>
I have run some numbers on previous versions of the code for
this. I'll take some new ones with the final code today/tomorrow
and post them.
Just wanted to let you know I saw this...
Thanks,
John
> Thanks
> -vlad
>
>
^ permalink raw reply
* Re: TCP performance regression
From: Eric Dumazet @ 2013-11-11 17:38 UTC (permalink / raw)
To: Felix Fietkau; +Cc: Sujith Manoharan, netdev, Dave Taht
In-Reply-To: <52810800.9020402@openwrt.org>
On Mon, 2013-11-11 at 17:38 +0100, Felix Fietkau wrote:
> On 2013-11-11 17:13, Sujith Manoharan wrote:
> > Eric Dumazet wrote:
> >> We have many choices.
> >>
> >> 1) Add back a minimum of ~128 K of outstanding bytes per TCP session,
> >> so that buggy drivers can sustain 'line rate'.
> >>
> >> Note that with 100 concurrent TCP streams, total amount of bytes
> >> queued on the NIC is 12 MB.
> >> And pfifo_fast qdisc will drop packets anyway.
> >>
> >> Thats what we call 'BufferBloat'
> >>
> >> 2) Try lower values like 64K. Still bufferbloat.
> >>
> >> 3) Fix buggy drivers, using a proper logic, or shorter timers (mvneta
> >> case for example)
> >>
> >> 4) Add a new netdev attribute, so that well behaving NIC drivers do not
> >> have to artificially force TCP stack to queue too many bytes in
> >> Qdisc/NIC queues.
> >
> > I think the quirks of 802.11 aggregation should be taken into account.
> > I am adding Felix to this thread, who would have more to say on latency/bufferbloat
> > with wireless drivers.
> I don't think this issue is about something as simple as timer handling
> for tx completion (or even broken/buggy drivers).
>
> There's simply no way to make 802.11 aggregation work well and have
> similar tx completion latency characteristics as Ethernet devices.
>
> 802.11 aggregation reduces the per-packet airtime overhead by combining
> multiple packets into one transmission (saving a lot of time getting a
> tx opportunity, transmitting the PHY header, etc.), which makes the
> 'line rate' heavily depend on the amount of buffering.
How long a TX packet is put on hold hoping a following packet will
come ?
> Aggregating multiple packets into one transmission also causes extra
> packet loss, which is compensated by retransmission and reordering, thus
> introducing additional latency.
>
> I don't think that TSQ can do a decent job of mitigating bufferbloat on
> 802.11n devices without a significant performance hit, so adding a new
> netdev attribute might be a good idea.
The netdev attribute would work, but might not work well if using a
tunnel...
^ permalink raw reply
* RE: TCP performance regression
From: Eric Dumazet @ 2013-11-11 17:41 UTC (permalink / raw)
To: David Laight; +Cc: Sujith Manoharan, netdev, Dave Taht
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B73FD@saturn3.aculab.com>
On Mon, 2013-11-11 at 16:35 +0000, David Laight wrote:
> > > It should be ok if the mac driver only gives the hardware a small
> > > number of bytes/packets - or one appropriate for the link speed.
> >
> > There is some confusion here.
> >
> > mvneta has a TX ring buffer, which can hold up to 532 TX descriptors.
> >
> > If this driver used skb_orphan(), a single TCP flow could use the whole
> > TX ring.
> >
> > TCP Small Queue would only limit the number of skbs on Qdisc.
> >
> > Try then to send a ping message, it will have to wait a lot.
>
> 532 is a ridiculously large number especially for a slow interface.
> At a guess you don't want more than 10-20ms of data in the tx ring.
> You might need extra descriptors for badly fragmented packets.
Thats why we invented BQL.
Problem is most driver authors don't care of the problem.
They already have hard time to make bug free drivers.
BQL is adding pressure and expose long standing bugs.
Some drivers have large TX rings to lower race probabilities.
^ 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