* [PATCH net-next 2/3] ipv6: increase maximum lifetime of flow labels
From: Florent Fourcot @ 2013-11-07 16:53 UTC (permalink / raw)
To: netdev; +Cc: Florent Fourcot
In-Reply-To: <1383843194-22945-1-git-send-email-florent.fourcot@enst-bretagne.fr>
If the last RFC 6437 does not give any constraints
for lifetime of flow labels, the previous RFC 3697
spoke of a minimum of 120 seconds between
reattribution of a flow label.
The maximum linger is currently set to 60 seconds
and does not allow this configuration without
CAP_NET_ADMIN right.
This patch increase the maximum linger to 150
seconds, allowing more flexibility to standard
users.
Signed-off-by: Florent Fourcot <florent.fourcot@enst-bretagne.fr>
---
net/ipv6/ip6_flowlabel.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv6/ip6_flowlabel.c b/net/ipv6/ip6_flowlabel.c
index 4a06ed0..5f10b0d 100644
--- a/net/ipv6/ip6_flowlabel.c
+++ b/net/ipv6/ip6_flowlabel.c
@@ -41,7 +41,7 @@
#define FL_MIN_LINGER 6 /* Minimal linger. It is set to 6sec specified
in old IPv6 RFC. Well, it was reasonable value.
*/
-#define FL_MAX_LINGER 60 /* Maximal linger timeout */
+#define FL_MAX_LINGER 150 /* Maximal linger timeout */
/* FL hash table */
--
1.8.4.rc3
^ permalink raw reply related
* [PATCH net-next v3 1/3] ipv6: enable IPV6_FLOWLABEL_MGR for getsockopt
From: Florent Fourcot @ 2013-11-07 16:53 UTC (permalink / raw)
To: netdev; +Cc: Florent Fourcot
It is already possible to set/put/renew a label
with IPV6_FLOWLABEL_MGR and setsockopt. This patch
add the possibility to get information about this
label (current value, time before expiration, etc).
It helps application to take decision for a renew
or a release of the label.
v2:
* Add spin_lock to prevent race condition
* return -ENOENT if no result found
* check if flr_action is GET
v3:
* move the spin_lock to protect only the
relevant code
Signed-off-by: Florent Fourcot <florent.fourcot@enst-bretagne.fr>
---
include/net/ipv6.h | 1 +
net/ipv6/ip6_flowlabel.c | 26 ++++++++++++++++++++++++++
net/ipv6/ipv6_sockglue.c | 28 ++++++++++++++++++++++++++++
3 files changed, 55 insertions(+)
diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index dd96638..2a5f668 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -250,6 +250,7 @@ struct ipv6_txoptions *fl6_merge_options(struct ipv6_txoptions *opt_space,
struct ipv6_txoptions *fopt);
void fl6_free_socklist(struct sock *sk);
int ipv6_flowlabel_opt(struct sock *sk, char __user *optval, int optlen);
+int ipv6_flowlabel_opt_get(struct sock *sk, struct in6_flowlabel_req *freq);
int ip6_flowlabel_init(void);
void ip6_flowlabel_cleanup(void);
diff --git a/net/ipv6/ip6_flowlabel.c b/net/ipv6/ip6_flowlabel.c
index 819578e..4a06ed0 100644
--- a/net/ipv6/ip6_flowlabel.c
+++ b/net/ipv6/ip6_flowlabel.c
@@ -475,6 +475,32 @@ static inline void fl_link(struct ipv6_pinfo *np, struct ipv6_fl_socklist *sfl,
spin_unlock_bh(&ip6_sk_fl_lock);
}
+int ipv6_flowlabel_opt_get(struct sock *sk, struct in6_flowlabel_req *freq)
+{
+ struct ipv6_pinfo *np = inet6_sk(sk);
+ struct ipv6_fl_socklist *sfl;
+
+ rcu_read_lock_bh();
+
+ for_each_sk_fl_rcu(np, sfl) {
+ if (sfl->fl->label == (np->flow_label & IPV6_FLOWLABEL_MASK)) {
+ spin_lock_bh(&ip6_fl_lock);
+ freq->flr_label = sfl->fl->label;
+ freq->flr_dst = sfl->fl->dst;
+ freq->flr_share = sfl->fl->share;
+ freq->flr_expires = (sfl->fl->expires - jiffies) / HZ;
+ freq->flr_linger = sfl->fl->linger / HZ;
+
+ spin_unlock_bh(&ip6_fl_lock);
+ rcu_read_unlock_bh();
+ return 0;
+ }
+ }
+ rcu_read_unlock_bh();
+
+ return -ENOENT;
+}
+
int ipv6_flowlabel_opt(struct sock *sk, char __user *optval, int optlen)
{
int uninitialized_var(err);
diff --git a/net/ipv6/ipv6_sockglue.c b/net/ipv6/ipv6_sockglue.c
index 4919a8e..1c6ce31 100644
--- a/net/ipv6/ipv6_sockglue.c
+++ b/net/ipv6/ipv6_sockglue.c
@@ -1212,6 +1212,34 @@ static int do_ipv6_getsockopt(struct sock *sk, int level, int optname,
val = np->sndflow;
break;
+ case IPV6_FLOWLABEL_MGR:
+ {
+ struct in6_flowlabel_req freq;
+
+ if (len < sizeof(freq))
+ return -EINVAL;
+
+ if (copy_from_user(&freq, optval, sizeof(freq)))
+ return -EFAULT;
+
+ if (freq.flr_action != IPV6_FL_A_GET)
+ return -EINVAL;
+
+ len = sizeof(freq);
+ memset(&freq, 0, sizeof(freq));
+
+ val = ipv6_flowlabel_opt_get(sk, &freq);
+ if (val < 0)
+ return val;
+
+ if (put_user(len, optlen))
+ return -EFAULT;
+ if (copy_to_user(optval, &freq, len))
+ return -EFAULT;
+
+ return 0;
+ }
+
case IPV6_ADDR_PREFERENCES:
val = 0;
--
1.8.4.rc3
^ permalink raw reply related
* [PATCH net-next v2 3/3] ipv6: protect flow label renew against GC
From: Florent Fourcot @ 2013-11-07 16:53 UTC (permalink / raw)
To: netdev; +Cc: Florent Fourcot
In-Reply-To: <1383843194-22945-1-git-send-email-florent.fourcot@enst-bretagne.fr>
Take ip6_fl_lock before to read and update
a label.
v2: protect only the relevant code
Reported-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
Signed-off-by: Florent Fourcot <florent.fourcot@enst-bretagne.fr>
---
net/ipv6/ip6_flowlabel.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/net/ipv6/ip6_flowlabel.c b/net/ipv6/ip6_flowlabel.c
index 5f10b0d..98fdcc6 100644
--- a/net/ipv6/ip6_flowlabel.c
+++ b/net/ipv6/ip6_flowlabel.c
@@ -345,6 +345,8 @@ static int fl6_renew(struct ip6_flowlabel *fl, unsigned long linger, unsigned lo
expires = check_linger(expires);
if (!expires)
return -EPERM;
+
+ spin_lock_bh(&ip6_fl_lock);
fl->lastuse = jiffies;
if (time_before(fl->linger, linger))
fl->linger = linger;
@@ -352,6 +354,8 @@ static int fl6_renew(struct ip6_flowlabel *fl, unsigned long linger, unsigned lo
expires = fl->linger;
if (time_before(fl->expires, fl->lastuse + expires))
fl->expires = fl->lastuse + expires;
+ spin_unlock_bh(&ip6_fl_lock);
+
return 0;
}
--
1.8.4.rc3
^ permalink raw reply related
* Re: [PATCH net-next v3 1/3] ipv6: enable IPV6_FLOWLABEL_MGR for getsockopt
From: Florent Fourcot @ 2013-11-07 16:56 UTC (permalink / raw)
To: netdev
In-Reply-To: <1383843194-22945-1-git-send-email-florent.fourcot@enst-bretagne.fr>
This series replaces the previous one, sent the 5th November.
Regards,
--
Florent
^ permalink raw reply
* Re: [PATCH net-next 3/8] bonding: add downdelay netlink support
From: Jay Vosburgh @ 2013-11-07 17:05 UTC (permalink / raw)
To: Scott Feldman; +Cc: vfalico, andy, netdev, shm
In-Reply-To: <20131107094308.15846.58301.stgit@monster-03.cumulusnetworks.com>
Scott Feldman <sfeldma@cumulusnetworks.com> wrote:
>Add IFLA_BOND_DOWNDELAY to allow get/set of bonding parameter
>downdelay via netlink.
>
>Signed-off-by: Scott Feldman <sfeldma@cumulusnetworks.com>
>---
> drivers/net/bonding/bond_netlink.c | 12 +++++++++++
> drivers/net/bonding/bond_options.c | 29 +++++++++++++++++++++++++++
> drivers/net/bonding/bond_sysfs.c | 38 +++++++++---------------------------
> drivers/net/bonding/bonding.h | 1 +
> include/uapi/linux/if_link.h | 1 +
> 5 files changed, 52 insertions(+), 29 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
>index 9ba5431..e684713 100644
>--- a/drivers/net/bonding/bond_netlink.c
>+++ b/drivers/net/bonding/bond_netlink.c
>@@ -26,6 +26,7 @@ static const struct nla_policy bond_policy[IFLA_BOND_MAX + 1] = {
> [IFLA_BOND_ACTIVE_SLAVE] = { .type = NLA_U32 },
> [IFLA_BOND_MIIMON] = { .type = NLA_U32 },
> [IFLA_BOND_UPDELAY] = { .type = NLA_U32 },
>+ [IFLA_BOND_DOWNDELAY] = { .type = NLA_U32 },
> };
>
> static int bond_validate(struct nlattr *tb[], struct nlattr *data[])
>@@ -85,6 +86,13 @@ static int bond_changelink(struct net_device *bond_dev,
> if (err)
> return err;
> }
>+ if (data[IFLA_BOND_DOWNDELAY]) {
>+ int downdelay = nla_get_u32(data[IFLA_BOND_DOWNDELAY]);
>+
>+ err = bond_option_downdelay_set(bond, downdelay);
>+ if (err)
>+ return err;
>+ }
> return 0;
> }
>
>@@ -106,6 +114,7 @@ static size_t bond_get_size(const struct net_device *bond_dev)
> nla_total_size(sizeof(u32)) + /* IFLA_BOND_ACTIVE_SLAVE */
> nla_total_size(sizeof(u32)) + /* IFLA_BOND_MIIMON */
> nla_total_size(sizeof(u32)) + /* IFLA_BOND_UPDELAY */
>+ nla_total_size(sizeof(u32)) + /* IFLA_BOND_DOWNDELAY */
> 0;
> }
>
>@@ -128,6 +137,9 @@ static int bond_fill_info(struct sk_buff *skb,
> if (nla_put_u32(skb, IFLA_BOND_UPDELAY, bond->params.updelay))
> goto nla_put_failure;
>
>+ if (nla_put_u32(skb, IFLA_BOND_DOWNDELAY, bond->params.downdelay))
>+ goto nla_put_failure;
>+
> return 0;
>
> nla_put_failure:
>diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
>index c0fea66..738be5f 100644
>--- a/drivers/net/bonding/bond_options.c
>+++ b/drivers/net/bonding/bond_options.c
>@@ -211,3 +211,32 @@ int bond_option_updelay_set(struct bonding *bond, int updelay)
>
> return 0;
> }
>+
>+int bond_option_downdelay_set(struct bonding *bond, int downdelay)
>+{
>+ if (!(bond->params.miimon)) {
>+ pr_err("%s: Unable to set down delay as MII monitoring is disabled\n", bond->dev->name);
>+ return -EPERM;
>+ }
I would argue that it is better to permit this option to be set
at any time, regardless of whether miimon is enabled or not. This
removes an ordering constraint and makes things generally easier to deal
with. Setting the option has no effect if miimon is not set, but that's
ok.
My comment here would also apply to other options that are
"secondary," in the sense that they affect the behavior of other options
or modes, e.g., updelay, arp_validate (in another path of this series),
arp_ip_targets, etc.
I'm aware that this is simply duplicating the existing behavior
of the sysfs API, but I'd be in favor of changing that, too. These
limitations are a holdover from the module paramters days, and should
have been made more flexible a long time ago.
-J
>+
>+ if (downdelay < 0) {
>+ pr_err("%s: Invalid down delay value %d not in range %d-%d; rejected.\n",
>+ bond->dev->name, downdelay, 0, INT_MAX);
>+ return -EINVAL;
>+ } else {
>+ if ((downdelay % bond->params.miimon) != 0) {
>+ pr_warn("%s: Warning: down delay (%d) is not a multiple of miimon (%d), delay rounded to %d ms\n",
>+ bond->dev->name, downdelay,
>+ bond->params.miimon,
>+ (downdelay / bond->params.miimon) *
>+ bond->params.miimon);
>+ }
>+ bond->params.downdelay = downdelay / bond->params.miimon;
>+ pr_info("%s: Setting down delay to %d.\n",
>+ bond->dev->name,
>+ bond->params.downdelay * bond->params.miimon);
>+
>+ }
>+
>+ return 0;
>+}
>diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
>index 1125bef..41a62ac 100644
>--- a/drivers/net/bonding/bond_sysfs.c
>+++ b/drivers/net/bonding/bond_sysfs.c
>@@ -706,42 +706,22 @@ static ssize_t bonding_store_downdelay(struct device *d,
> struct device_attribute *attr,
> const char *buf, size_t count)
> {
>- int new_value, ret = count;
>+ int new_value, ret;
> struct bonding *bond = to_bond(d);
>
>- if (!(bond->params.miimon)) {
>- pr_err("%s: Unable to set down delay as MII monitoring is disabled\n",
>- bond->dev->name);
>- ret = -EPERM;
>- goto out;
>- }
>-
> if (sscanf(buf, "%d", &new_value) != 1) {
> pr_err("%s: no down delay value specified.\n", bond->dev->name);
>- ret = -EINVAL;
>- goto out;
>+ return -EINVAL;
> }
>- if (new_value < 0) {
>- pr_err("%s: Invalid down delay value %d not in range %d-%d; rejected.\n",
>- bond->dev->name, new_value, 0, INT_MAX);
>- ret = -EINVAL;
>- goto out;
>- } else {
>- if ((new_value % bond->params.miimon) != 0) {
>- pr_warning("%s: Warning: down delay (%d) is not a multiple of miimon (%d), delay rounded to %d ms\n",
>- bond->dev->name, new_value,
>- bond->params.miimon,
>- (new_value / bond->params.miimon) *
>- bond->params.miimon);
>- }
>- bond->params.downdelay = new_value / bond->params.miimon;
>- pr_info("%s: Setting down delay to %d.\n",
>- bond->dev->name,
>- bond->params.downdelay * bond->params.miimon);
>
>- }
>+ if (!rtnl_trylock())
>+ return restart_syscall();
>
>-out:
>+ ret = bond_option_downdelay_set(bond, new_value);
>+ if (!ret)
>+ ret = count;
>+
>+ rtnl_unlock();
> return ret;
> }
> static DEVICE_ATTR(downdelay, S_IRUGO | S_IWUSR,
>diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
>index 408e6c2..40987f3 100644
>--- a/drivers/net/bonding/bonding.h
>+++ b/drivers/net/bonding/bonding.h
>@@ -430,6 +430,7 @@ int bond_option_mode_set(struct bonding *bond, int mode);
> int bond_option_active_slave_set(struct bonding *bond, struct net_device *slave_dev);
> int bond_option_miimon_set(struct bonding *bond, int miimon);
> int bond_option_updelay_set(struct bonding *bond, int updelay);
>+int bond_option_downdelay_set(struct bonding *bond, int downdelay);
> struct net_device *bond_option_active_slave_get_rcu(struct bonding *bond);
> struct net_device *bond_option_active_slave_get(struct bonding *bond);
>
>diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
>index 361414e..a372831 100644
>--- a/include/uapi/linux/if_link.h
>+++ b/include/uapi/linux/if_link.h
>@@ -333,6 +333,7 @@ enum {
> IFLA_BOND_ACTIVE_SLAVE,
> IFLA_BOND_MIIMON,
> IFLA_BOND_UPDELAY,
>+ IFLA_BOND_DOWNDELAY,
> __IFLA_BOND_MAX,
> };
>
>
---
-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
^ permalink raw reply
* Re: [PATCH] net: x86: bpf: don't forget to free sk_filter (v2)
From: Alexei Starovoitov @ 2013-11-07 17:17 UTC (permalink / raw)
To: Andrey Vagin; +Cc: netdev, linux-kernel, Eric Dumazet, David S. Miller
In-Reply-To: <1383798912-10832-1-git-send-email-avagin@openvz.org>
On Wed, Nov 6, 2013 at 8:35 PM, Andrey Vagin <avagin@openvz.org> wrote:
> sk_filter isn't freed if bpf_func is equal to sk_run_filter.
>
> This memory leak was introduced by v3.12-rc3-224-gd45ed4a4
> "net: fix unsafe set_memory_rw from softirq".
>
> Before this patch sk_filter was freed in sk_filter_release_rcu,
> now it should be freed in bpf_jit_free.
>
> Here is output of kmemleak:
> unreferenced object 0xffff8800b774eab0 (size 128):
> comm "systemd", pid 1, jiffies 4294669014 (age 124.062s)
> hex dump (first 32 bytes):
> 00 00 00 00 0b 00 00 00 20 63 7f b7 00 88 ff ff ........ c......
> 60 d4 55 81 ff ff ff ff 30 d9 55 81 ff ff ff ff `.U.....0.U.....
> backtrace:
> [<ffffffff816444be>] kmemleak_alloc+0x4e/0xb0
> [<ffffffff811845af>] __kmalloc+0xef/0x260
> [<ffffffff81534028>] sock_kmalloc+0x38/0x60
> [<ffffffff8155d4dd>] sk_attach_filter+0x5d/0x190
> [<ffffffff815378a1>] sock_setsockopt+0x991/0x9e0
> [<ffffffff81531bd6>] SyS_setsockopt+0xb6/0xd0
> [<ffffffff8165f3e9>] system_call_fastpath+0x16/0x1b
> [<ffffffffffffffff>] 0xffffffffffffffff
>
> v2: add extra { } after else
>
> Fixes: d45ed4a4e33a ("net: fix unsafe set_memory_rw from softirq")
> Acked-by: Daniel Borkmann <dborkman@redhat.com>
> Cc: Alexei Starovoitov <ast@plumgrid.com>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Signed-off-by: Andrey Vagin <avagin@openvz.org>
> ---
Acked-by: Alexei Starovoitov <ast@plumgrid.com>
Thanks!
^ permalink raw reply
* Re: [PATCH RESEND] packet: Deliver VLAN TPID to userspace
From: Atzm Watanabe @ 2013-11-07 17:22 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: Ben Hutchings, David Miller, netdev, stephen
In-Reply-To: <5277E890.20308@redhat.com>
At Mon, 04 Nov 2013 19:33:52 +0100,
Daniel Borkmann wrote:
>
> On 11/04/2013 06:16 PM, Ben Hutchings wrote:
> > On Wed, 2013-10-30 at 16:03 +0900, Atzm Watanabe wrote:
> > [...]
> >> Should it be structures as below?
> >>
> >> struct tpacket_hdr_variant1 {
> >> __u32 tp_rxhash;
> >> __u32 tp_vlan_tci;
> >> };
> >>
> >> struct tpacket_hdr_variant2 {
> >> __u32 tp_rxhash;
> >> __u32 tp_vlan_tci;
> >> __u32 tp_vlan_tpid;
> >> };
> >>
> >> struct tpacket3_hdr {
> >> __u32 tp_next_offset;
> >> __u32 tp_sec;
> >> __u32 tp_nsec;
> >> __u32 tp_snaplen;
> >> __u32 tp_len;
> >> __u32 tp_status;
> >> __u16 tp_mac;
> >> __u16 tp_net;
> >> /* pkt_hdr variants */
> >> union {
> >> struct tpacket_hdr_variant1 hv1;
> >> struct tpacket_hdr_variant2 hv2;
> >> };
> >> };
> >>
> >> If it's ok, I'd like to send the patch v2.
> > [...]
> >
> > I think this makes sense.
> >
> > Also add a BUILD_BUG_ON(TPACKET3_HDRLEN != 48) somewhere.
>
> Sorry for coming late into this discussion.
>
> I think the packet_mmap API with its 3 different API versions
> already is a "bit" terrible, and should only be further extended
> for user space if there is a _*huge*_ (!) improvement somewhere
> (e.g. in terms of packet capturing/transmission performance) that
> would justify it. I'm thinking that this could e.g. be in terms
> of packet capturing and transmission performance.
>
> Otherwise, each time we want to add a new member, we add yet
> another subheader for userspace into tpacket, making it even
> more complicated to use, and adding more "legacy" API fragments?
Thank you for the comments.
Indeed, your worry will become reality if we often want to add new
members as other aims, but I also think that VLAN related members
perhaps won't be added anymore because the VLAN only contains TCI
and ethertype.
> To solve your problem, why don't you add a socket option that,
> if _enabled_, would reconstruct the vlan header as it was seen
> on the "wire" and push that up to userspace, just like libpcap
> does internally. It's not perfect either, but perhaps a better
> way to go. What do you think?
Sounds good to me, but I also think that a socket option should be
added when we want to add new members to enable userspace to
reassemble the pakcet for reasons other than the VLAN. Because TCI
is already put into the tpacket headers or auxdata, userspace may
want to get TPID by same way.
However, if we cannot add new members into the header anyway,
we need to avoid that by other ways like you proposed.
Thanks!
^ permalink raw reply
* [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.
From: David Laight @ 2013-11-07 17:20 UTC (permalink / raw)
To: netdev, linux-usb, Sarah Sharp
Section 4.11.7.1 of rev 1.0 of the xhci specification states that a link TRB
can only occur at a boundary between underlying USB frames (512 bytes for 480M).
If this isn't done the USB frames aren't formatted correctly and, for example,
the USB3 ethernet ax88179_178a card will stop sending (while still receiving)
when running a netperf tcp transmit test with (say) and 8k buffer.
While this change improves things a lot (it runs for 20 minutes rather than
a few seconds), there is still something else wrong.
This should be a candidate for stable, the ax88179_178a driver defaults to
gso and tso enabled so it passes a lot of fragmented skb to the USB stack.
Signed-off-by: David Laight <david.laight@aculab.com>
---
Although I've got a USB2 analyser its trigger and trace stuff is almost
unusable! In any case this fails much faster on USB3 (Intel i7 cpu).
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 5480215..23abc9b 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2927,8 +2932,43 @@ static int prepare_ring(struct xhci_hcd *xhci, struct xhci_ring *ep_ring,
}
while (1) {
- if (room_on_ring(xhci, ep_ring, num_trbs))
- break;
+ if (room_on_ring(xhci, ep_ring, num_trbs)) {
+ union xhci_trb *trb = ep_ring->enqueue;
+ unsigned int usable = ep_ring->enq_seg->trbs +
+ TRBS_PER_SEGMENT - 1 - trb;
+ u32 nop_cmd;
+
+ /*
+ * Section 4.11.7.1 TD Fragments states that a link
+ * TRB must only occur at the boundary between
+ * data bursts (eg 512 bytes for 480M).
+ * While it is possible to split a large fragment
+ * we don't know the size yet.
+ * Simplest solution is to fill the trb before the
+ * LINK with nop commands.
+ */
+ if (num_trbs == 1 || num_trbs <= usable || usable == 0)
+ break;
+
+ if (num_trbs >= TRBS_PER_SEGMENT) {
+ xhci_err(xhci, "Too many fragments %d, max %d\n",
+ num_trbs, TRBS_PER_SEGMENT - 1);
+ return -ENOMEM;
+ }
+
+ nop_cmd = cpu_to_le32(TRB_TYPE(TRB_TR_NOOP) |
+ ep_ring->cycle_state);
+ for (; !TRB_TYPE_LINK_LE32(trb->link.control); trb++) {
+ trb->generic.field[0] = 0;
+ trb->generic.field[1] = 0;
+ trb->generic.field[2] = 0;
+ trb->generic.field[3] = nop_cmd;
+ ep_ring->num_trbs_free--;
+ }
+ ep_ring->enqueue = trb;
+ if (room_on_ring(xhci, ep_ring, num_trbs))
+ break;
+ }
if (ep_ring == xhci->cmd_ring) {
xhci_err(xhci, "Do not support expand command ring\n");
^ permalink raw reply related
* [PATCH] Fix "ip rule delete table 256"
From: Andreas Henriksson @ 2013-11-07 17:26 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, shemminger
When trying to delete a table >= 256 using iproute2 the local table
will be deleted.
The table id is specified as a netlink attribute when it needs more then
8 bits and iproute2 then sets the table field to RT_TABLE_UNSPEC (0).
Preconditions to matching the table id in the rule delete code
doesn't seem to take the "table id in netlink attribute" into condition
so the frh_get_table helper function never gets to do its job when
matching against current rule.
Use the helper function twice instead of peaking at the table value directly.
Originally reported at: http://bugs.debian.org/724783
Reported-by: Nicolas HICHER <nhicher@avencall.com>
Signed-off-by: Andreas Henriksson <andreas@fatal.se>
---
net/core/fib_rules.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
An alternative would be to say RT_TABLE_UNSPEC is incorrect value
to use for the rule->table when using the netlink attribute and
change iproute2 instead....
... but RT_TABLE_UNSPEC sounds quite suitable, so why not
change the kernel to be more forgiving.
diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c
index 2e65413..f409e0b 100644
--- a/net/core/fib_rules.c
+++ b/net/core/fib_rules.c
@@ -460,7 +460,8 @@ static int fib_nl_delrule(struct sk_buff *skb, struct nlmsghdr* nlh)
if (frh->action && (frh->action != rule->action))
continue;
- if (frh->table && (frh_get_table(frh, tb) != rule->table))
+ if (frh_get_table(frh, tb) &&
+ (frh_get_table(frh, tb) != rule->table))
continue;
if (tb[FRA_PRIORITY] &&
--
1.8.4.2
^ permalink raw reply related
* Re: [3/3] gso: Handle malicious GRO packets without crashing
From: Sergei Shtylyov @ 2013-11-07 19:13 UTC (permalink / raw)
To: Herbert Xu, Eric Dumazet
Cc: Ben Hutchings, David Miller, christoph.paasch, netdev, hkchu,
mwdalton
In-Reply-To: <20131107070847.GC31638@gondor.apana.org.au>
Hello.
On 11/07/2013 10:08 AM, Herbert Xu wrote:
> As virtio_net can now generate GRO frag_list packets without
> sufficient verification, we need to handle malicious GRO packets
> thrown at us.
> This patch converts to affected BUG_ONs in skb_segment to rate-
> limited warnings.
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index bcc3f1c..fb1106d 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -2881,7 +2881,15 @@ struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features)
> while (tail->next)
> tail = tail->next;
>
> - BUG_ON(fskb && tail->len != len + doffset);
> + if (fskb && tail->len != len + doffset) {
> + net_warn_ratelimited(
> + "skb_segment: "
> + "illegal GSO fragment: %u %u\n",
Don't break up the message -- chekpatch.pl should allow that...
> @@ -2929,7 +2937,15 @@ struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features)
> if (pos < offset + len) {
> struct sk_buff *fskb2 = fskb;
>
> - BUG_ON(pos + fskb->len != offset + len);
> + if (pos + fskb->len != offset + len) {
> + net_warn_ratelimited(
> + "skb_segment: "
> + "illegal GSO trailer: %u %u\n",
Same here.
WBR, Sergei
^ permalink raw reply
* Re: [2/3] gso: Handle new frag_list of frags GRO packets
From: Ben Hutchings @ 2013-11-07 18:16 UTC (permalink / raw)
To: Herbert Xu
Cc: Eric Dumazet, David Miller, christoph.paasch, netdev, hkchu,
mwdalton
In-Reply-To: <20131107070647.GB31638@gondor.apana.org.au>
On Thu, 2013-11-07 at 15:06 +0800, Herbert Xu wrote:
> Recently GRO started generating packets with frag_lists of frags.
> This was not handled by GSO, thus leading to a crash.
>
> Thankfully these packets are of a regular form and are easy to
> handle. This patch handles them by calling skb_segment for each
> frag_list entry. The depth of recursion is limited to just one.
>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
>
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 88b7dc6..bcc3f1c 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
[...]
> @@ -2855,8 +2853,40 @@ struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features)
> nskb->data - tnl_hlen,
> doffset + tnl_hlen);
>
> - if (fskb != skb_shinfo(skb)->frag_list)
> - goto perform_csum_check;
> + if (fskb != skb_shinfo(skb)->frag_list) {
> + struct sk_buff *nsegs;
> +
> + if (nskb->len == len + doffset)
> + goto perform_csum_check;
> +
> + SKB_FRAG_ASSERT(nskb);
> +
> + __skb_pull(nskb, doffset);
> + skb_shinfo(nskb)->gso_size = mss;
> + nsegs = skb_segment(nskb, features);
> +
> + err = PTR_ERR(nsegs);
> + if (IS_ERR(nsegs)) {
> + kfree(nskb);
Should be kfree_skb().
> + goto err;
> + }
> + err = -ENOMEM;
It would be clearer to put this in front of the 'err' label:
err_nomem:
err = -ENOMEM;
and change the allocation failure paths to goto err_nomem.
> + if (segs)
> + tail->next = nsegs;
> + else
> + segs = nsegs;
> +
> + tail = nsegs;
> + while (tail->next)
> + tail = tail->next;
> +
> + BUG_ON(fskb && tail->len != len + doffset);
Deserves a comment, maybe?
> + len = nskb->len;
> + kfree(nskb);
> + continue;
> + }
>
> if (!sg) {
> nskb->ip_summed = CHECKSUM_NONE;
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [3/3] gso: Handle malicious GRO packets without crashing
From: Ben Hutchings @ 2013-11-07 18:18 UTC (permalink / raw)
To: Herbert Xu
Cc: Eric Dumazet, David Miller, christoph.paasch, netdev, hkchu,
mwdalton
In-Reply-To: <20131107070847.GC31638@gondor.apana.org.au>
On Thu, 2013-11-07 at 15:08 +0800, Herbert Xu wrote:
> As virtio_net can now generate GRO frag_list packets without
> sufficient verification, we need to handle malicious GRO packets
> thrown at us.
>
> This patch converts to affected BUG_ONs in skb_segment to rate-
> limited warnings.
>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
>
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index bcc3f1c..fb1106d 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -2881,7 +2881,15 @@ struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features)
> while (tail->next)
> tail = tail->next;
>
> - BUG_ON(fskb && tail->len != len + doffset);
Oh well, disregard my previous request for a comment.
> + if (fskb && tail->len != len + doffset) {
> + net_warn_ratelimited(
> + "skb_segment: "
> + "illegal GSO fragment: %u %u\n",
> + tail->len, len + doffset);
> + kfree(nskb);
kfree_skb()
> + err = -EINVAL;
> + goto err;
> + }
>
> len = nskb->len;
> kfree(nskb);
> @@ -2929,7 +2937,15 @@ struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features)
> if (pos < offset + len) {
> struct sk_buff *fskb2 = fskb;
>
> - BUG_ON(pos + fskb->len != offset + len);
> + if (pos + fskb->len != offset + len) {
> + net_warn_ratelimited(
> + "skb_segment: "
> + "illegal GSO trailer: %u %u\n",
> + pos + fskb->len, offset + len);
> + kfree(nskb);
kfree_skb()
> + err = -EINVAL;
> + goto err;
> + }
>
> pos += fskb->len;
> fskb = fskb->next;
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: ipv6: a question about ECMP
From: sowmini varadhan @ 2013-11-07 18:32 UTC (permalink / raw)
To: Duan Jiong, David Miller, nicolas.dichtel, netdev
In-Reply-To: <20131107121602.GI8144@order.stressinduktion.org>
On Thu, Nov 7, 2013 at 7:16 AM, Hannes Frederic Sowa
<hannes@stressinduktion.org> wrote:
> Hi Duan!
>
> On Thu, Nov 07, 2013 at 06:33:20PM +0800, Duan Jiong wrote:
>> After reading the ip6_pol_route(), i have a question about ECMP. Why we call
>> the rt6_multipath_select() after calling rt6_select()?
>> In my opinion, the route returned by rt6_select() has a highest score, but the route
>> returned by rt6_multipath_select() may has a lower score than the former, because the
>> ECMP don't take the route preference into consideration. That means that the kernel will
>> choose a less-desirable route.
>
> ECMP routes only differ in the gateway the specify, so I doubt there will be
> any change in the score they woud receive. rt6_multipath_select does merly
> make sure we don't select the same route again and again.
rt6_multipath_select() -> rt6_socre_route() seems to require that the
interface *must* matchi, which is consistent with your assertion above that
"ECMP routes differ in gw only".
But for IPv6, the gw addr is a a link-local, which is only required to be
unique on the link. Thus, e.g., you can have fe80::1 as the gw on both eth0 and
eth1.
What is the assumption around "cost" for ECMP here- are we assuming some
form of link bundling (Section 6 of rfc 2991) here? or is the "multiple parallel
links" case handled somewhere else, that I am missing?
--Sowmini
>
> Please note, the rt6_info's siblings fields were added for the solely purpose
> of ECMP and the insertion only updates the siblings list if the above criteria
> did hold. They make sure the routes lookup up do differ on each lookup, so it
> does actually do multipath and does not depend on the order the routes where
> inserted.
>
> Hope that helps,
>
> Hannes
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH net] netlink: fix netlink_ack with large messages
From: Jiri Benc @ 2013-11-07 18:57 UTC (permalink / raw)
To: netdev; +Cc: Pablo Neira Ayuso
Commit c05cdb1b864f ("netlink: allow large data transfers from user-space")
does not handle cases where netlink_ack is used to report an error. In such
case, the original message is copied to the ack message, which needs to be
large enough.
Signed-off-by: Jiri Benc <jbenc@redhat.com>
---
net/netlink/af_netlink.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 8df7f64..090f624 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -1809,7 +1809,7 @@ out_put:
sock_put(sk);
out:
#endif
- return alloc_skb(size, gfp_mask);
+ return netlink_alloc_large_skb(size, gfp_mask);
}
EXPORT_SYMBOL_GPL(netlink_alloc_skb);
--
1.7.6.5
^ permalink raw reply related
* [PATCH net-next] nfnetlink: do not ack malformed messages
From: Jiri Benc @ 2013-11-07 18:59 UTC (permalink / raw)
To: netdev; +Cc: Pablo Neira Ayuso, Petr Matousek
Commit 0628b123c96d ("netfilter: nfnetlink: add batch support and use it
from nf_tables") introduced a bug leading to various crashes in netlink_ack
when netlink message with invalid nlmsg_len was sent by an unprivileged
user.
Signed-off-by: Jiri Benc <jbenc@redhat.com>
---
net/netfilter/nfnetlink.c | 8 +++++---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/net/netfilter/nfnetlink.c b/net/netfilter/nfnetlink.c
index 027f16a..046aa13 100644
--- a/net/netfilter/nfnetlink.c
+++ b/net/netfilter/nfnetlink.c
@@ -363,13 +363,15 @@ static void nfnetlink_rcv(struct sk_buff *skb)
struct net *net = sock_net(skb->sk);
int msglen;
- if (!ns_capable(net->user_ns, CAP_NET_ADMIN))
- return netlink_ack(skb, nlh, -EPERM);
-
if (nlh->nlmsg_len < NLMSG_HDRLEN ||
skb->len < nlh->nlmsg_len)
return;
+ if (!ns_capable(net->user_ns, CAP_NET_ADMIN)) {
+ netlink_ack(skb, nlh, -EPERM);
+ return;
+ }
+
if (nlh->nlmsg_type == NFNL_MSG_BATCH_BEGIN) {
struct nfgenmsg *nfgenmsg;
--
1.7.6.5
^ permalink raw reply related
* Re: [PATCH] wcn36xx: Fix logging macro with unnecessary semicolon
From: Joe Perches @ 2013-11-07 19:40 UTC (permalink / raw)
To: Eugene Krasnikov
Cc: Luis R. Rodriguez, John W. Linville, wcn36xx, linux-wireless,
netdev, linux-kernel@vger.kernel.org, ath5k-devel, ath9k-devel,
ath10k, Kalle Valo, wil6210
In-Reply-To: <CAFSJ42Y-3BZqj+JRNa8wdxjnm1fguD8ToH5Oj0m56yT5NexdVQ@mail.gmail.com>
On Thu, 2013-11-07 at 07:32 +0000, Eugene Krasnikov wrote:
> Hi Joe,
Hi Eugene.
> I personally like the idea of making some kind of framework on top of
> printing because all ath drivers are using the same printing approach
> and combining all that code in one place will reduce amount of code in
> each particular driver. As a true engineer i like when it's less code
> = less work to do = less bugs:)
>
> Suggestion is to send a patch with semicolon fix only
It's a one line fix, and pretty trivial to do yourself if you
want it.
I do think though the slightly odd wcn36xx_<level> uses of:
-#define wcn36xx_err(fmt, arg...) \
- printk(KERN_ERR pr_fmt("ERROR " fmt), ##arg);
-
-#define wcn36xx_warn(fmt, arg...) \
- printk(KERN_WARNING pr_fmt("WARNING " fmt), ##arg)
-
-#define wcn36xx_info(fmt, arg...) \
- printk(KERN_INFO pr_fmt(fmt), ##arg)
-
[]
+#define wcn36xx_err(fmt, ...) \
+ pr_err("ERROR " fmt, ##__VA_ARGS__)
+#define wcn36xx_warn(fmt, ...) \
+ pr_warn("WARNING " fmt, ##__VA_ARGS__)
+#define wcn36xx_info(fmt, ...) \
+ pr_info(fmt, ##__VA_ARGS__)
should still be converted to more normal pr_<level> like I did.
I don't believe that prefixing of "ERROR " and "WARNING " to
pr_err and pr_warn actually gain anything as the same level
information can be got already with dmesg -r via the "<3>" and
"<4>" prefixes or dmesg -l3 -l4
> have a
> second round of convincing ath guys to change printing code. How does
> that sound?
Luis? Kalle? Other Qualcomm/Atheros folk?
One of the nominal benefits of separating the ath_<level>
macros by subsystem was perf/tracing. I believe some of the
ath subsystems have integrated the dmesg and tracing code.
I'm not sure how separable those may be here or if it's even
desirable to separate them. I'd guess not.
^ permalink raw reply
* ethtool 3.12 released
From: Ben Hutchings @ 2013-11-07 19:56 UTC (permalink / raw)
To: netdev
ethtool version 3.12 has been released.
Home page: https://ftp.kernel.org/pub/software/network/ethtool/
Download link:
https://ftp.kernel.org/pub/software/network/ethtool/ethtool-3.12.tar.xz
Release notes:
* Fix: Remove alternate method to check for VLAN tag offload on Linux
< 2.6.37 (-k/-K options)
* Fix: Hide state of VLAN tag offload and LRO if the kernel is too old
for us to reliably detect them (-k option)
* Feature: Add register dump support for Solarflare SFC9100 family
(-d option)
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* [PATCH ethtool] sfc: Remove preprocessor condition from register table definitions
From: Ben Hutchings @ 2013-11-07 19:58 UTC (permalink / raw)
To: netdev
This condition was mistakenly copied from the corresponding
definitions in the out-of-tree version of sfc. It has no effect here.
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
I've applied this change just after the 3.12 tag.
Ben.
sfc.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/sfc.c b/sfc.c
index 1bb3689..9478b38 100644
--- a/sfc.c
+++ b/sfc.c
@@ -3739,9 +3739,7 @@ static const struct efx_nic_reg_table efx_nic_reg_tables[] = {
REGISTER_TABLE_CZ(MC_TREG_SMEM),
/* MSIX_PBA_TABLE is not mapped */
/* SRM_DBG is not mapped (and is redundant with BUF_FLL_TBL) */
-#if !defined(EFX_USE_KCOMPAT) || defined(EFX_HAVE_VMALLOC_REG_DUMP_BUF)
REGISTER_TABLE_BZ(RX_FILTER_TBL0),
-#endif
REGISTER_TABLE_DZ(BIU_MC_SFT_STATUS),
};
--
1.8.1.4
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply related
* Re: [BUG] v3.12-rc5-139-gbdeeab6 assertion failed at drivers/net/bonding/bond_main.c (3398)
From: David Miller @ 2013-11-07 20:12 UTC (permalink / raw)
To: vfalico; +Cc: thomas, netdev, fubar, andy, nikolay, robbat2
In-Reply-To: <20131107142418.GL19702@redhat.com>
From: Veaceslav Falico <vfalico@redhat.com>
Date: Thu, 7 Nov 2013 15:24:18 +0100
> Sorry for bringing this up - completely my fault. There are further
> reports
> of this bug in mainline. I've read the
> ./Documentation/networking/netdev-FAQ.txt but haven't seen a way to
> get a
> fix that got in net-next to the (correct) net tree.
>
> Is there any way to do it?
>
> Sorry again for the mess :-(.
Since we're in the merge window the tree going to Linus next would
be net-next, and the fix is there.
Once it hits Linus's tree I can queue it up for -stable.
^ permalink raw reply
* Re: [PATCH net-next 0/2] bonding: extend round-robin mode
From: David Miller @ 2013-11-07 20:12 UTC (permalink / raw)
To: nikolay; +Cc: netdev, andy, fubar, vfalico
In-Reply-To: <1383655902-18744-1-git-send-email-nikolay@redhat.com>
From: Nikolay Aleksandrov <nikolay@redhat.com>
Date: Tue, 5 Nov 2013 13:51:40 +0100
> This small patchset adds a new option called packets_per_slave to the
> bonding which aims to extend round-robin mode with the following effects:
> 0 - choose the slave id at random
> 1 - packet per slave (standard round-robin, default option value)
> >1 - transmit >1 packets per slave, switch the slaves in round-robin
> Patch02 adds a description for the new option to the bonding documentation.
Series applied, thanks.
^ permalink raw reply
* Re: [BUG] v3.12-rc5-139-gbdeeab6 assertion failed at drivers/net/bonding/bond_main.c (3398)
From: Veaceslav Falico @ 2013-11-07 20:18 UTC (permalink / raw)
To: David Miller; +Cc: thomas, netdev, fubar, andy, nikolay, robbat2
In-Reply-To: <20131107.151212.489598950578675166.davem@davemloft.net>
On Thu, Nov 07, 2013 at 03:12:12PM -0500, David Miller wrote:
>From: Veaceslav Falico <vfalico@redhat.com>
>Date: Thu, 7 Nov 2013 15:24:18 +0100
>
>> Sorry for bringing this up - completely my fault. There are further
>> reports
>> of this bug in mainline. I've read the
>> ./Documentation/networking/netdev-FAQ.txt but haven't seen a way to
>> get a
>> fix that got in net-next to the (correct) net tree.
>>
>> Is there any way to do it?
>>
>> Sorry again for the mess :-(.
>
>Since we're in the merge window the tree going to Linus next would
>be net-next, and the fix is there.
>
>Once it hits Linus's tree I can queue it up for -stable.
Awesome, please do, thank you.
And sorry again for the noise.
>--
>To unsubscribe from this list: send the line "unsubscribe netdev" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* no new features at this time
From: David Miller @ 2013-11-07 20:32 UTC (permalink / raw)
To: netdev
We are in the merge window and Linus is going to start taking pull
requests in next week.
This means there should be no new feature additions being submitted
right now.
John has a wireless pull request pending for me, but that is the only
new thing I want to see showing up in patchwork at this time.
I am going to reject and delete from patchwork any patches which
are not appropriate until after the merge window closes and
net-next reopens.
Thanks in advance for your cooperation.
^ permalink raw reply
* Re: [PATCH v2] phy: Add MOXA MDIO driver
From: David Miller @ 2013-11-07 20:37 UTC (permalink / raw)
To: jonas.jensen
Cc: netdev, linux-arm-kernel, linux-kernel, devicetree, f.fainelli,
grant.likely
In-Reply-To: <1383666901-22354-1-git-send-email-jonas.jensen@gmail.com>
From: Jonas Jensen <jonas.jensen@gmail.com>
Date: Tue, 5 Nov 2013 16:55:01 +0100
> The MOXA UC-711X hardware(s) has an ethernet controller that seem
> to be developed internally. The IC used is "RTL8201CP".
>
> This patch adds an MDIO driver which handles the MII bus.
>
> Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
Applied, thanks Jonas.
^ permalink raw reply
* Re: [PATCH v4 net-next] net: introduce dev_set_forwarding()
From: David Miller @ 2013-11-07 21:17 UTC (permalink / raw)
To: bhutchings
Cc: eric.dumazet, christoph.paasch, herbert, netdev, hkchu, mwdalton
In-Reply-To: <1383584130.1553.2.camel@bwh-desktop.uk.level5networks.com>
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Mon, 4 Nov 2013 16:55:30 +0000
> On Sat, 2013-11-02 at 12:58 -0700, Eric Dumazet wrote:
>> From: Eric Dumazet <edumazet@google.com>
>>
>> Christoph Paasch and Jerry Chu reported crashes in skb_segment() caused
>> by commit 8a29111c7ca6 ("net: gro: allow to build full sized skb")
>>
>> skb_segment() only deals with a frag_list chain containing MSS sized
>> fragments. Even if we fix this problem, its better if GRO layer
>> doesn't build skb with a frag_list in the first place, to let TSO
>> packets reaching output devices.
>>
>> David Miller and Ben Hutchings suggested we keep track of number of
>> forwarding users to be able to :
>>
>> - Disable LRO
>> - Make sure GRO layer do not use skb frag_list to extend skb capacity
>>
>> Note that after this patch, LRO is automatically re-enabled if
>> forwarding is disabled on the device, or if a device is removed
>> from a bridge.
> [...]
>
> Reviewed-by: Ben Hutchings <bhutchings@solarflare.com>
Applied, thanks everyone.
^ permalink raw reply
* Re: [PATCH v2 2/2] x86: add prefetching to do_csum
From: Neil Horman @ 2013-11-07 21:23 UTC (permalink / raw)
To: Andi Kleen
Cc: linux-kernel, sebastien.dugue, Thomas Gleixner, Ingo Molnar,
H. Peter Anvin, x86, netdev
In-Reply-To: <87iow58eqf.fsf@tassilo.jf.intel.com>
On Wed, Nov 06, 2013 at 12:19:52PM -0800, Andi Kleen wrote:
> Neil Horman <nhorman@tuxdriver.com> writes:
>
> > do_csum was identified via perf recently as a hot spot when doing
> > receive on ip over infiniband workloads. After alot of testing and
> > ideas, we found the best optimization available to us currently is to
> > prefetch the entire data buffer prior to doing the checksum
>
> On what CPU? Most modern CPUs should not have any trouble at all
> prefetching a linear access.
>
> Also for large buffers it is unlikely that all the prefetches
> are actually executed, there is usually some limit.
>
> As a minimum you would need:
> - run it with a range of buffer sizes
> - run this on a range of different CPUs and show no major regressions
> - describe all of this actually in the description
>
> But I find at least this patch very dubious.
>
> -Andi
>
Well, if you look back in the thread, you can see several tests done with
various forms of prefetching, that show performance improvements, but if you
want them all collected, heres what I have, using the perf bench from patch 1.
As you can see, you're right, on newer hardware theres negligible advantage (but
no regression that I can see). On older hardware however, we see a definate
improvement (up to 3%). I'm afraid I don't have a wide variety of hardware
handy at the moment to do any large scale testing on multiple cpu's. But if you
have them available, please share your results
Regards
Neil
vendor_id : AuthenticAMD
cpu family : 16
model : 8
model name : AMD Opteron(tm) Processor 4130
stepping : 0
microcode : 0x10000da
cpu MHz : 800.000
cache size : 512 KB
physical id : 1
siblings : 4
core id : 3
cpu cores : 4
apicid : 11
initial apicid : 11
fpu : yes
fpu_exception : yes
cpuid level : 5
wp : yes
flags : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov
pat pse36 clflush mmx fxsr sse sse2 ht syscall nx mmxext fxsr_opt pdpe1gb rdtscp
lm 3dnowext 3dnow constant_tsc rep_good nopl nonstop_tsc extd_apicid pni monitor
cx16 popcnt lahf_lm cmp_legacy svm extapic cr8_legacy abm sse4a misalignsse
3dnowprefetch osvw ibs skinit wdt nodeid_msr hw_pstate npt lbrv svm_lock
nrip_save pausefilter
bogomips : 5200.49
TLB size : 1024 4K pages
clflush size : 64
cache_alignment : 64
address sizes : 48 bits physical, 48 bits virtual
power management: ts ttp tm stc 100mhzsteps hwpstate
Without prefecth:
length | Set Sz| iterations | cycles/byte
1500B | 64MB | 1000000 | 1.432338
1500B | 128MB | 1000000 | 1.426212
1500B | 256MB | 1000000 | 1.425988
1500B | 512MB | 1000000 | 1.517873
9000B | 64MB | 1000000 | 0.897998
9000B | 128MB | 1000000 | 0.884120
9000B | 256MB | 1000000 | 0.881770
9000B | 512MB | 1000000 | 0.883644
64KB | 64MB | 1000000 | 0.813054
64KB | 128MB | 1000000 | 0.801859
64KB | 256MB | 1000000 | 0.796415
64KB | 512MB | 1000000 | 0.793869
With prefetch:
length | Set Sz| iterations | cycles/byte
1500B | 64MB | 1000000 | 1.442855
1500B | 128MB | 1000000 | 1.438841
1500B | 256MB | 1000000 | 1.427324
1500B | 512MB | 1000000 | 1.462715
9000B | 64MB | 1000000 | 0.894097
9000B | 128MB | 1000000 | 0.884738
9000B | 256MB | 1000000 | 0.881370
9000B | 512MB | 1000000 | 0.884799
64KB | 64MB | 1000000 | 0.813512
64KB | 128MB | 1000000 | 0.801596
64KB | 256MB | 1000000 | 0.795575
64KB | 512MB | 1000000 | 0.793927
==========================================================================================
vendor_id : GenuineIntel
cpu family : 6
model : 42
model name : Intel(R) Core(TM) i7-2600 CPU @ 3.40GHz
stepping : 7
microcode : 0x29
cpu MHz : 2754.000
cache size : 8192 KB
physical id : 0
siblings : 8
core id : 3
cpu cores : 4
apicid : 7
initial apicid : 7
fpu : yes
fpu_exception : yes
cpuid level : 13
wp : yes
flags : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov
pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx rdtscp lm
constant_tsc arch_perfmon pebs bts rep_good nopl xtopology nonstop_tsc
aperfmperf eagerfpu pni pclmulqdq dtes64 monitor ds_cpl vmx smx est tm2 ssse3
cx16 xtpr pdcm pcid sse4_1 sse4_2 x2apic popcnt tsc_deadline_timer aes xsave avx
lahf_lm ida arat epb xsaveopt pln pts dtherm tpr_shadow vnmi flexpriority ept
vpid
bogomips : 6784.46
clflush size : 64
cache_alignment : 64
address sizes : 36 bits physical, 48 bits virtual
Without prefetch:
length | Set Sz| iterations | cycles/byte
1500B | 64MB | 1000000 | 1.343645
1500B | 128MB | 1000000 | 1.345782
1500B | 256MB | 1000000 | 1.353145
1500B | 512MB | 1000000 | 1.354844
9000B | 64MB | 1000000 | 0.856552
9000B | 128MB | 1000000 | 0.852786
9000B | 256MB | 1000000 | 0.854705
9000B | 512MB | 1000000 | 0.863308
64KB | 64MB | 1000000 | 0.771888
64KB | 128MB | 1000000 | 0.773453
64KB | 256MB | 1000000 | 0.771728
64KB | 512MB | 1000000 | 0.771390
With prefetching:
length | Set Sz| iterations | cycles/byte
1500B | 64MB | 1000000 | 1.344733
1500B | 128MB | 1000000 | 1.342285
1500B | 256MB | 1000000 | 1.344818
1500B | 512MB | 1000000 | 1.342632
9000B | 64MB | 1000000 | 0.851043
9000B | 128MB | 1000000 | 0.850629
9000B | 256MB | 1000000 | 0.852207
9000B | 512MB | 1000000 | 0.851927
64KB | 64MB | 1000000 | 0.768549
64KB | 128MB | 1000000 | 0.768623
64KB | 256MB | 1000000 | 0.768938
64KB | 512MB | 1000000 | 0.768824
==========================================================================================
vendor_id : AuthenticAMD
cpu family : 16
model : 9
model name : AMD Opteron(tm) Processor 6172
stepping : 1
microcode : 0x10000d9
cpu MHz : 800.000
cache size : 512 KB
physical id : 1
siblings : 12
core id : 5
cpu cores : 12
apicid : 43
initial apicid : 27
fpu : yes
fpu_exception : yes
cpuid level : 5
wp : yes
flags : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov
pat pse36 clflush mmx fxsr sse sse2 ht syscall nx mmxext fxsr_opt pdpe1gb rdtscp
lm 3dnowext 3dnow constant_tsc rep_good nopl nonstop_tsc extd_apicid amd_dcm pni
monitor cx16 popcnt lahf_lm cmp_legacy svm extapic cr8_legacy abm sse4a
misalignsse 3dnowprefetch osvw ibs skinit wdt nodeid_msr hw_pstate npt lbrv
svm_lock nrip_save pausefilter
bogomips : 4189.63
TLB size : 1024 4K pages
clflush size : 64
cache_alignment : 64
address sizes : 48 bits physical, 48 bits virtual
power management: ts ttp tm stc 100mhzsteps hwpstate
Without prefetch:
length | Set Sz| iterations | cycles/byte
1500B | 64MB | 1000000 | 1.415370
1500B | 128MB | 1000000 | 1.437025
1500B | 256MB | 1000000 | 1.424822
1500B | 512MB | 1000000 | 1.442021
9000B | 64MB | 1000000 | 0.891699
9000B | 128MB | 1000000 | 0.884261
9000B | 256MB | 1000000 | 0.880179
9000B | 512MB | 1000000 | 0.882190
64KB | 64MB | 1000000 | 0.813047
64KB | 128MB | 1000000 | 0.800755
64KB | 256MB | 1000000 | 0.795207
64KB | 512MB | 1000000 | 0.792065
With prefetch:
length | Set Sz| iterations | cycles/byte
1500B | 64MB | 1000000 | 1.424003
1500B | 128MB | 1000000 | 1.435567
1500B | 256MB | 1000000 | 1.446858
1500B | 512MB | 1000000 | 1.459407
9000B | 64MB | 1000000 | 0.899858
9000B | 128MB | 1000000 | 0.885170
9000B | 256MB | 1000000 | 0.883936
9000B | 512MB | 1000000 | 0.886158
64KB | 64MB | 1000000 | 0.814136
64KB | 128MB | 1000000 | 0.802202
64KB | 256MB | 1000000 | 0.796140
64KB | 512MB | 1000000 | 0.793792
==========================================================================================
processor : 0
vendor_id : AuthenticAMD
cpu family : 6
model : 10
model name : AMD Athlon(tm) XP 2800+
stepping : 0
cpu MHz : 2079.461
cache size : 512 KB
fdiv_bug : no
f00f_bug : no
coma_bug : no
fpu : yes
fpu_exception : yes
cpuid level : 1
wp : yes
flags : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov
pat pse36 mmx fxsr sse syscall mmxext 3dnowext 3dnow
bogomips : 4158.92
clflush size : 32
cache_alignment : 32
address sizes : 34 bits physical, 32 bits virtual
power management: ts
Without prefetch:
length | Set Sz| iterations | cycles/byte
1500B | 64MB | 1000000 | 3.335217
1500B | 128MB | 1000000 | 3.403103
1500B | 256MB | 1000000 | 3.445059
1500B | 512MB | 1000000 | 3.742008
9000B | 64MB | 1000000 | 47.466255
9000B | 128MB | 1000000 | 47.742751
9000B | 256MB | 1000000 | 47.965001
9000B | 512MB | 1000000 | 48.589349
64KB | 64MB | 1000000 | 118.088638
64KB | 128MB | 1000000 | 118.261744
64KB | 256MB | 1000000 | 118.349641
64KB | 512MB | 1000000 | 118.695321
With prefetch:
length | Set Sz| iterations | cycles/byte
1500B | 64MB | 1000000 | 3.231086
1500B | 128MB | 1000000 | 3.423485
1500B | 256MB | 1000000 | 3.278899
1500B | 512MB | 1000000 | 3.545504
9000B | 64MB | 1000000 | 46.907795
9000B | 128MB | 1000000 | 47.321743
9000B | 256MB | 1000000 | 47.306189
9000B | 512MB | 1000000 | 48.144320
64KB | 64MB | 1000000 | 117.897735
64KB | 128MB | 1000000 | 118.122266
64KB | 256MB | 1000000 | 118.126397
64KB | 512MB | 1000000 | 118.546901
^ 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