Netdev List
 help / color / mirror / Atom feed
* [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

* 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

* 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 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-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

* [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

* [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 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] net: flow_dissector: small optimizations in IPv4 dissect
From: Eric Dumazet @ 2013-11-07 16:37 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

From: Eric Dumazet <edumazet@google.com>

By moving code around, we avoid :

1) A reload of iph->ihl (bit field, so needs a mask)

2) A conditional test (replaced by a conditional mov on x86)
   Fast path loads iph->protocol anyway.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/core/flow_dissector.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 0242035192f1..d6ef17322500 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -68,13 +68,13 @@ ip:
 		iph = skb_header_pointer(skb, nhoff, sizeof(_iph), &_iph);
 		if (!iph || iph->ihl < 5)
 			return false;
+		nhoff += iph->ihl * 4;
 
+		ip_proto = iph->protocol;
 		if (ip_is_fragment(iph))
 			ip_proto = 0;
-		else
-			ip_proto = iph->protocol;
+
 		iph_to_flow_copy_addrs(flow, iph);
-		nhoff += iph->ihl * 4;
 		break;
 	}
 	case __constant_htons(ETH_P_IPV6): {

^ permalink raw reply related

* Re: [patch] net: make ndev->irq signed for error handling
From: Mugunthan V N @ 2013-11-07 16:33 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: David S. Miller, netdev, linux-kernel, kernel-janitors
In-Reply-To: <20131107122249.GN20521@mwanda>

On Thursday 07 November 2013 05:52 PM, Dan Carpenter wrote:
> On Thu, Nov 07, 2013 at 05:44:38PM +0530, Mugunthan V N wrote:
>> On Thursday 07 November 2013 01:18 PM, Dan Carpenter wrote:
>>> There is a bug in cpsw_probe() where we do:
>>>
>>> 	ndev->irq = platform_get_irq(pdev, 0);
>>> 	if (ndev->irq < 0) {
>>>
>>> The problem is that "ndev->irq" is unsigned so the error handling
>>> doesn't work.  I have changed it to a regular int.
>>>
>>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>> ndev->irq is never used any where in the driver, I think its better to
>> remove this part of code from probe. If every one is ok, I can send a
>> patch to remove the code.
> It seems like cpsw_ndo_poll_controller() uses it.
>
>

That can be changed to pass one of the interrupt numbers from priv as
the irq number is not used in interrupt service routine.

Regards
Mugunthan V N

^ permalink raw reply

* Re: Using HTB over MultiQ
From: Eric Dumazet @ 2013-11-07 16:13 UTC (permalink / raw)
  To: Sergey Popovich; +Cc: netdev
In-Reply-To: <15059450.XJdZrMIbNO@tuxracer>

On Thu, 2013-11-07 at 15:49 +0200, Sergey Popovich wrote:


> commit 64153ce0a7b61b2a5cacb01805cbf670142339e9
> Author: Eric Dumazet
> Date:   Thu Jun 6 14:53:16 2013 -0700
> 
>     net_sched: htb: do not setup default rate estimators
> 
> rate estimators do not setup by default. To enable rate estimators
> you could try to load module sch_htb with htb_rate_est=1 
> or echo 1 >/sys/module/sch_htb/parameters/htb_rate_est and recreate
> hierarchy.
> 

To check if packets went through a class, you do not need
a rate estimator. Its 0 anyway after an idle period.

You have to take a look at counters as in :

Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)

^ permalink raw reply

* Re: Using HTB over MultiQ
From: Eric Dumazet @ 2013-11-07 16:10 UTC (permalink / raw)
  To: John Fastabend; +Cc: John Fastabend, Anton 'EvilMan' Danilov, netdev
In-Reply-To: <20131107151727.GA31116@nitbit.x32>

On Thu, 2013-11-07 at 07:17 -0800, John Fastabend wrote:

> I think this is all it would take I only compile tested this for now.
> If its useful I could send out a real (tested) patch later today.

Well, this might be useful, my question was more 'Is it currently
supported', thanks !

^ permalink raw reply

* Re: Using HTB over MultiQ
From: John Fastabend @ 2013-11-07 15:17 UTC (permalink / raw)
  To: John Fastabend; +Cc: Eric Dumazet, Anton 'EvilMan' Danilov, netdev
In-Reply-To: <527BAC8E.1090005@intel.com>

On Thu, Nov 07, 2013 at 07:06:54AM -0800, John Fastabend wrote:
> On 11/7/2013 6:54 AM, Eric Dumazet wrote:
> >On Thu, 2013-11-07 at 06:39 -0800, John Fastabend wrote:
> >
> >>
> >>With the multiq qdisc you could attach filter to the root qdisc and use
> >>skbedit to set the queue_mapping field,
> >>
> >>#tc filter add dev eth0 parent 1: protocol ip prio 1 u32 \
> >>	match ip dst 192.168.0.3 \
> >>	action skbedit queue_mapping 3
> >>
> >
> >Oh right, this is the way ;)
> >
> >I wonder if we can have 'action skbedit rxhash 34' ?
> >
> 
> Sure, it should easy enough.

I think this is all it would take I only compile tested this for now.
If its useful I could send out a real (tested) patch later today.

diff --git a/include/net/tc_act/tc_skbedit.h b/include/net/tc_act/tc_skbedit.h
index e103fe0..3951f7d 100644
--- a/include/net/tc_act/tc_skbedit.h
+++ b/include/net/tc_act/tc_skbedit.h
@@ -27,6 +27,7 @@ struct tcf_skbedit {
 	u32			flags;
 	u32     		priority;
 	u32     		mark;
+	u32			rxhash;
 	u16			queue_mapping;
 	/* XXX: 16-bit pad here? */
 };
diff --git a/include/uapi/linux/tc_act/tc_skbedit.h b/include/uapi/linux/tc_act/tc_skbedit.h
index 7a2e910..d5a1d55 100644
--- a/include/uapi/linux/tc_act/tc_skbedit.h
+++ b/include/uapi/linux/tc_act/tc_skbedit.h
@@ -27,6 +27,7 @@
 #define SKBEDIT_F_PRIORITY		0x1
 #define SKBEDIT_F_QUEUE_MAPPING		0x2
 #define SKBEDIT_F_MARK			0x4
+#define SKBEDIT_F_RXHASH		0x8
 
 struct tc_skbedit {
 	tc_gen;
@@ -39,6 +40,7 @@ enum {
 	TCA_SKBEDIT_PRIORITY,
 	TCA_SKBEDIT_QUEUE_MAPPING,
 	TCA_SKBEDIT_MARK,
+	TCA_SKBEDIT_RXHASH,
 	__TCA_SKBEDIT_MAX
 };
 #define TCA_SKBEDIT_MAX (__TCA_SKBEDIT_MAX - 1)
diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c
index cb42211..f6b6820 100644
--- a/net/sched/act_skbedit.c
+++ b/net/sched/act_skbedit.c
@@ -55,6 +55,8 @@ static int tcf_skbedit(struct sk_buff *skb, const struct tc_action *a,
 		skb_set_queue_mapping(skb, d->queue_mapping);
 	if (d->flags & SKBEDIT_F_MARK)
 		skb->mark = d->mark;
+	if (d->flags & SKBEDIT_F_RXHASH)
+		skb->rxhash = d->rxhash;
 
 	spin_unlock(&d->tcf_lock);
 	return d->tcf_action;
@@ -65,6 +67,7 @@ static const struct nla_policy skbedit_policy[TCA_SKBEDIT_MAX + 1] = {
 	[TCA_SKBEDIT_PRIORITY]		= { .len = sizeof(u32) },
 	[TCA_SKBEDIT_QUEUE_MAPPING]	= { .len = sizeof(u16) },
 	[TCA_SKBEDIT_MARK]		= { .len = sizeof(u32) },
+	[TCA_SKBEDIT_RXHASH]		= { .len = sizeof(u32) },
 };
 
 static int tcf_skbedit_init(struct net *net, struct nlattr *nla,
@@ -75,7 +78,7 @@ static int tcf_skbedit_init(struct net *net, struct nlattr *nla,
 	struct tc_skbedit *parm;
 	struct tcf_skbedit *d;
 	struct tcf_common *pc;
-	u32 flags = 0, *priority = NULL, *mark = NULL;
+	u32 flags = 0, *priority = NULL, *mark = NULL, *rxhash = NULL;
 	u16 *queue_mapping = NULL;
 	int ret = 0, err;
 
@@ -104,6 +107,11 @@ static int tcf_skbedit_init(struct net *net, struct nlattr *nla,
 		mark = nla_data(tb[TCA_SKBEDIT_MARK]);
 	}
 
+	if (tb[TCA_SKBEDIT_RXHASH] != NULL) {
+		flags |= SKBEDIT_F_RXHASH;
+		rxhash = nla_data(tb[TCA_SKBEDIT_RXHASH]);
+	}
+
 	if (!flags)
 		return -EINVAL;
 
@@ -135,6 +143,8 @@ static int tcf_skbedit_init(struct net *net, struct nlattr *nla,
 		d->queue_mapping = *queue_mapping;
 	if (flags & SKBEDIT_F_MARK)
 		d->mark = *mark;
+	if (flags & SKBEDIT_F_RXHASH)
+		d->rxhash = *rxhash;
 
 	d->tcf_action = parm->action;
 
@@ -181,6 +191,9 @@ static int tcf_skbedit_dump(struct sk_buff *skb, struct tc_action *a,
 	    nla_put(skb, TCA_SKBEDIT_MARK, sizeof(d->mark),
 		    &d->mark))
 		goto nla_put_failure;
+	if ((d->flags & SKBEDIT_F_RXHASH) &&
+	    nla_put(skb, TCA_SKBEDIT_RXHASH, sizeof(d->rxhash),
+		    &d->rxhash))
 	t.install = jiffies_to_clock_t(jiffies - d->tcf_tm.install);
 	t.lastuse = jiffies_to_clock_t(jiffies - d->tcf_tm.lastuse);
 	t.expires = jiffies_to_clock_t(d->tcf_tm.expires);

^ permalink raw reply related

* [PATCH] netdev: smc91x: enable for xtensa
From: Baruch Siach @ 2013-11-07 15:16 UTC (permalink / raw)
  To: netdev; +Cc: Matthew Davey, Baruch Siach

Tested in VLAB Works Xtensa simulation.

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 drivers/net/ethernet/smsc/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/smsc/Kconfig b/drivers/net/ethernet/smsc/Kconfig
index 068fc44..753630f 100644
--- a/drivers/net/ethernet/smsc/Kconfig
+++ b/drivers/net/ethernet/smsc/Kconfig
@@ -6,7 +6,7 @@ config NET_VENDOR_SMSC
 	bool "SMC (SMSC)/Western Digital devices"
 	default y
 	depends on ARM || ISA || MAC || ARM64 || MIPS || M32R || SUPERH || \
-		BLACKFIN || MN10300 || COLDFIRE || PCI || PCMCIA
+		BLACKFIN || MN10300 || COLDFIRE || XTENSA || PCI || PCMCIA
 	---help---
 	  If you have a network (Ethernet) card belonging to this class, say Y
 	  and read the Ethernet-HOWTO, available from
@@ -39,7 +39,7 @@ config SMC91X
 	select CRC32
 	select MII
 	depends on (ARM || M32R || SUPERH || MIPS || BLACKFIN || \
-		    MN10300 || COLDFIRE || ARM64)
+		    MN10300 || COLDFIRE || ARM64 || XTENSA)
 	---help---
 	  This is a driver for SMC's 91x series of Ethernet chipsets,
 	  including the SMC91C94 and the SMC91C111. Say Y if you want it
-- 
1.8.4.rc3

^ permalink raw reply related

* Re: Using HTB over MultiQ
From: John Fastabend @ 2013-11-07 15:06 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Anton 'EvilMan' Danilov, netdev
In-Reply-To: <1383836068.9412.71.camel@edumazet-glaptop2.roam.corp.google.com>

On 11/7/2013 6:54 AM, Eric Dumazet wrote:
> On Thu, 2013-11-07 at 06:39 -0800, John Fastabend wrote:
>
>>
>> With the multiq qdisc you could attach filter to the root qdisc and use
>> skbedit to set the queue_mapping field,
>>
>> #tc filter add dev eth0 parent 1: protocol ip prio 1 u32 \
>> 	match ip dst 192.168.0.3 \
>> 	action skbedit queue_mapping 3
>>
>
> Oh right, this is the way ;)
>
> I wonder if we can have 'action skbedit rxhash 34' ?
>

Sure, it should easy enough.

>
>> if you configure the filters to map to the correct classes this would
>> work.
>>
>> Or another way would be use mqprio and steer packets to HTB classes
>> using the skb->priority. The priority can be set by iptables/nftables
>> or an ingress filter.
>
> Yes, but this might duplicate the 'customer' tree Anton has to put on
> the filters anyway ?
>

hmm not sure I understand. As long as customer flows are mapped to the
correct HTB qdisc via skb priority with the correct htb child classes
what would be duplicated?

^ permalink raw reply

* Re: Using HTB over MultiQ
From: Sergey Popovich @ 2013-11-07 13:49 UTC (permalink / raw)
  To: netdev
In-Reply-To: <CAEzD07+Haf-Kg52H7GsxC5XiQn2KXozHbxgL4Lvn0grhFXVFmA@mail.gmail.com>


> But I see zero statistics on the leaf htb classes and nonzero
> statistics on the classifier filters:
> 
> ~$ tc -s -p filter list dev eth1
>  ...
>  filter parent 10: protocol ip pref 5 u32 fh 2:f2:800 order 2048 key
> ht 2 bkt f2 flowid 11:1736  (rule hit 306 success 306)
>    match IP src xx.xx.xx.xx/30 (success 306 )
>  ...
> 
> ~$ tc -s -s -d c ls dev eth1 classid 11:1736
>  class htb 11:1736 parent 11:1 leaf 1736: prio 0 quantum 12800 rate
> 1024Kbit ceil 1024Kbit burst 1599b/1 mpu 0b overhead 0b cburst 1599b/1
> mpu 0b overhead 0b level 0
>   Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
>   rate 0bit 0pps backlog 0b 0p requeues 0
>   lended: 0 borrowed: 0 giants: 0
>   tokens: 195312 ctokens: 195312
> 
> I think I've lost from view the some aspects of settings.
> Has anyone setuped the like complex scheme over the multiq discipline?

Since

commit 64153ce0a7b61b2a5cacb01805cbf670142339e9
Author: Eric Dumazet
Date:   Thu Jun 6 14:53:16 2013 -0700

    net_sched: htb: do not setup default rate estimators

rate estimators do not setup by default. To enable rate estimators
you could try to load module sch_htb with htb_rate_est=1 
or echo 1 >/sys/module/sch_htb/parameters/htb_rate_est and recreate
hierarchy.

-- 
SP5474-RIPE
Sergey Popovich

^ permalink raw reply

* Re: Using HTB over MultiQ
From: Eric Dumazet @ 2013-11-07 14:54 UTC (permalink / raw)
  To: John Fastabend; +Cc: Anton 'EvilMan' Danilov, netdev
In-Reply-To: <527BA63F.7040900@intel.com>

On Thu, 2013-11-07 at 06:39 -0800, John Fastabend wrote:

> 
> With the multiq qdisc you could attach filter to the root qdisc and use
> skbedit to set the queue_mapping field,
> 
> #tc filter add dev eth0 parent 1: protocol ip prio 1 u32 \
> 	match ip dst 192.168.0.3 \
> 	action skbedit queue_mapping 3
> 

Oh right, this is the way ;)

I wonder if we can have 'action skbedit rxhash 34' ?


> if you configure the filters to map to the correct classes this would
> work.
> 
> Or another way would be use mqprio and steer packets to HTB classes
> using the skb->priority. The priority can be set by iptables/nftables
> or an ingress filter.

Yes, but this might duplicate the 'customer' tree Anton has to put on
the filters anyway ?

^ permalink raw reply

* Re: [PATCH net-next v2 3/3] net_sched: Use pr_debug replace printk(KERN_DEBUG ...)
From: Sergei Shtylyov @ 2013-11-07 14:43 UTC (permalink / raw)
  To: Yang Yingliang, davem, netdev; +Cc: eric.dumazet, jhs, stephen
In-Reply-To: <1383790412-41944-4-git-send-email-yangyingliang@huawei.com>

Hello.

On 07-11-2013 6:13, Yang Yingliang wrote:

> Replace printk(KERN_DEBUG ...) with pr_debug() and

    Note that this is not an equivalent change: first variant always prints 
the message, while pr_debug() only does this if DEBUG is #define'd or dynamic 
debugging is enabled.

> replace pr_warning() with pr_warn().

    This should probably be in a separate patch.

> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
> ---
>   net/sched/sch_cbq.c    | 2 +-
>   net/sched/sch_dsmark.c | 2 +-
>   net/sched/sch_gred.c   | 4 ++--
>   net/sched/sch_htb.c    | 6 +++---
>   4 files changed, 7 insertions(+), 7 deletions(-)

> diff --git a/net/sched/sch_cbq.c b/net/sched/sch_cbq.c
> index a8f40f5..9e3a9dc 100644
> --- a/net/sched/sch_cbq.c
> +++ b/net/sched/sch_cbq.c
> @@ -1060,7 +1060,7 @@ static void cbq_normalize_quanta(struct cbq_sched_data *q, int prio)
>   			}
>   			if (cl->quantum <= 0 ||
>   				cl->quantum > 32*qdisc_dev(cl->qdisc)->mtu) {
> -				pr_warning("CBQ: class %08x has bad quantum==%ld, repaired.\n",
> +				pr_warn("CBQ: class %08x has bad quantum==%ld, repaired.\n",
>   					   cl->common.classid, cl->quantum);

    You should also adjust indentation of the continuation line. Same comment 
to the code furhter below...

[...]
> diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
> index f6e8a74..6586f3b 100644
> --- a/net/sched/sch_htb.c
> +++ b/net/sched/sch_htb.c

> @@ -1484,13 +1484,13 @@ static int htb_change_class(struct Qdisc *sch, u32 classid,
>   	if (!cl->level) {
>   		cl->quantum = hopt->rate.rate / q->rate2quantum;
>   		if (!hopt->quantum && cl->quantum < 1000) {
> -			pr_warning(
> +			pr_warn(
>   			       "HTB: quantum of class %X is small. Consider r2q change.\n",

    It would have been good if you moved the string literal to the preceding 
line. Same comment to the below code.

WBR, Sergei

^ permalink raw reply

* Re: Using HTB over MultiQ
From: John Fastabend @ 2013-11-07 14:39 UTC (permalink / raw)
  To: Eric Dumazet, Anton 'EvilMan' Danilov; +Cc: netdev
In-Reply-To: <1383834021.9412.61.camel@edumazet-glaptop2.roam.corp.google.com>

On 11/7/2013 6:20 AM, Eric Dumazet wrote:
> On Thu, 2013-11-07 at 06:11 -0800, Eric Dumazet wrote:
>> On Thu, 2013-11-07 at 17:12 +0400, Anton 'EvilMan' Danilov wrote:
>>> Hello.
>>>
>>> I'm experimenting with high performance linux router with 10G NICs.
>>> On high traffic rates the performance are limited by the lock of root
>>> queue discipline. For avoid impact of locking i've decided to build
>>> QoS scheme over the multiq qdisc.
>>>
>
> Oh well, this is a router.
>
> So I think you need to change the device queue selection so that it only
> depends on your filters / htb classes.
>
> Otherwise flows for a particular 'customer' might be spread on the 8
> queues, so the rate could be 8 * 1Mbit, instead of 1Mbit.
>


With the multiq qdisc you could attach filter to the root qdisc and use
skbedit to set the queue_mapping field,

#tc filter add dev eth0 parent 1: protocol ip prio 1 u32 \
	match ip dst 192.168.0.3 \
	action skbedit queue_mapping 3

if you configure the filters to map to the correct classes this would
work.

Or another way would be use mqprio and steer packets to HTB classes
using the skb->priority. The priority can be set by iptables/nftables
or an ingress filter.

.John

^ permalink raw reply

* Re: [PATCH net-next v2 2/3] net_sched: fix some checkpatch errors
From: Sergei Shtylyov @ 2013-11-07 14:33 UTC (permalink / raw)
  To: Yang Yingliang, davem, netdev; +Cc: eric.dumazet, jhs, stephen
In-Reply-To: <1383790412-41944-3-git-send-email-yangyingliang@huawei.com>

Hello.

On 07-11-2013 6:13, Yang Yingliang wrote:

> There are some checkpatch errors, fix them.

> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
> Suggested-by: Stephen Hemminger <stephen@networkplumber.org>
[...]

> diff --git a/net/sched/act_api.c b/net/sched/act_api.c
> index fd70728..d92a90e9 100644
> --- a/net/sched/act_api.c
> +++ b/net/sched/act_api.c
> @@ -191,7 +191,8 @@ u32 tcf_hash_new_index(u32 *idx_gen, struct tcf_hashinfo *hinfo)
>   			val = 1;
>   	} while (tcf_hash_lookup(val, hinfo));
>
> -	return (*idx_gen = val);
> +	*idx_gen = val;
> +	return *idx_gen;

	return val;

would have been simpler.

> diff --git a/net/sched/sch_cbq.c b/net/sched/sch_cbq.c
> index 7a42c81..a8f40f5 100644
> --- a/net/sched/sch_cbq.c
> +++ b/net/sched/sch_cbq.c
> @@ -1058,7 +1058,8 @@ static void cbq_normalize_quanta(struct cbq_sched_data *q, int prio)
>   				cl->quantum = (cl->weight*cl->allot*q->nclasses[prio])/
>   					q->quanta[prio];
>   			}
> -			if (cl->quantum <= 0 || cl->quantum>32*qdisc_dev(cl->qdisc)->mtu) {
> +			if (cl->quantum <= 0 ||
> +				cl->quantum > 32*qdisc_dev(cl->qdisc)->mtu) {

    According to the networking coding style, the continuation line should 
start right under 'cl' on the first line of *if*. The way you did it makes it 
harder for the eyes to differentiate the code in the *if* branch from the *if* 
expression.

>   				pr_warning("CBQ: class %08x has bad quantum==%ld, repaired.\n",
>   					   cl->common.classid, cl->quantum);
>   				cl->quantum = qdisc_dev(cl->qdisc)->mtu/2 + 1;
[...]

WBR, Sergei

^ permalink raw reply

* Re: Using HTB over MultiQ
From: Eric Dumazet @ 2013-11-07 14:29 UTC (permalink / raw)
  To: Anton 'EvilMan' Danilov; +Cc: netdev
In-Reply-To: <CAEzD07+Haf-Kg52H7GsxC5XiQn2KXozHbxgL4Lvn0grhFXVFmA@mail.gmail.com>

On Thu, 2013-11-07 at 17:12 +0400, Anton 'EvilMan' Danilov wrote:

> But I see zero statistics on the leaf htb classes and nonzero
> statistics on the classifier filters:
> 
> ~$ tc -s -p filter list dev eth1
>  ...
>  filter parent 10: protocol ip pref 5 u32 fh 2:f2:800 order 2048 key
> ht 2 bkt f2 flowid 11:1736  (rule hit 306 success 306)
>    match IP src xx.xx.xx.xx/30 (success 306 )
>  ...

Filter matches and flowid 11:1736 chosen

But then, when the packet enters HTB, it enters HTB on one of the 8 HTB
trees, and only one of them contains 11:1736 and could queue the packet
into its local pfifo 50.

For 7 others HTB trees, flowid 11:1736 doesn't match any htb class, so
packet is queued into the default queue attached to the HTB tree.

> 
> ~$ tc -s -s -d c ls dev eth1 classid 11:1736
>  class htb 11:1736 parent 11:1 leaf 1736: prio 0 quantum 12800 rate
> 1024Kbit ceil 1024Kbit burst 1599b/1 mpu 0b overhead 0b cburst 1599b/1
> mpu 0b overhead 0b level 0
>   Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
>   rate 0bit 0pps backlog 0b 0p requeues 0
>   lended: 0 borrowed: 0 giants: 0
>   tokens: 195312 ctokens: 195312

^ 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 14:24 UTC (permalink / raw)
  To: David Miller; +Cc: thomas, netdev, fubar, andy, nikolay, robbat2
In-Reply-To: <20131019.184411.2017497732906948451.davem@davemloft.net>

On Sat, Oct 19, 2013 at 06:44:11PM -0400, David Miller wrote:
>From: Veaceslav Falico <vfalico@redhat.com>
>Date: Sat, 19 Oct 2013 22:52:22 +0200
>
>> Fixed in commit b32418705107265dfca5edfe2b547643e53a732e ("bonding:
>> RCUify
>> bond_set_rx_mode()") net-next. It should get into mainline soon.
>
>If it's in net-next it's not going into mainline soon.
>
>A change that fixes a bug should go into 'net', not 'net-next'.

Hi David,

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 :-(.

Thank you!

^ permalink raw reply

* Re: [PATCH net-next v2 1/3] net_sched: tbf: support of 64bit rates
From: Sergei Shtylyov @ 2013-11-07 14:25 UTC (permalink / raw)
  To: Yang Yingliang, davem, netdev; +Cc: eric.dumazet, jhs, stephen
In-Reply-To: <1383790412-41944-2-git-send-email-yangyingliang@huawei.com>

Hello.

On 07-11-2013 6:13, Yang Yingliang wrote:

> With psched_ratecfg_precompute(), tbf can deal with 64bit rates.
> Add two new attributes so that tc can use them to break the 32bit
> limit.

> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
[...]

> diff --git a/net/sched/sch_tbf.c b/net/sched/sch_tbf.c
> index b057122..b736517 100644
> --- a/net/sched/sch_tbf.c
> +++ b/net/sched/sch_tbf.c
[...]
> @@ -402,6 +409,13 @@ static int tbf_dump(struct Qdisc *sch, struct sk_buff *skb)
>   	opt.buffer = PSCHED_NS2TICKS(q->buffer);
>   	if (nla_put(skb, TCA_TBF_PARMS, sizeof(opt), &opt))
>   		goto nla_put_failure;
> +	if ((q->rate.rate_bytes_ps >= (1ULL << 32)) &&
> +			nla_put_u64(skb, TCA_TBF_RATE64, q->rate.rate_bytes_ps))
> +		goto nla_put_failure;
> +	if (q->peak_present &&
> +			(q->peak.rate_bytes_ps >= (1ULL << 32)) &&
> +			nla_put_u64(skb, TCA_TBF_PRATE64, q->peak.rate_bytes_ps))
> +		goto nla_put_failure;

    According to the networking coding style, the *if* continuation lines 
should start under the next character after (.

WBR, Sergei

^ permalink raw reply

* Re: Using HTB over MultiQ
From: Eric Dumazet @ 2013-11-07 14:20 UTC (permalink / raw)
  To: Anton 'EvilMan' Danilov; +Cc: netdev
In-Reply-To: <1383833480.9412.58.camel@edumazet-glaptop2.roam.corp.google.com>

On Thu, 2013-11-07 at 06:11 -0800, Eric Dumazet wrote:
> On Thu, 2013-11-07 at 17:12 +0400, Anton 'EvilMan' Danilov wrote:
> > Hello.
> > 
> > I'm experimenting with high performance linux router with 10G NICs.
> > On high traffic rates the performance are limited by the lock of root
> > queue discipline. For avoid impact of locking i've decided to build
> > QoS scheme over the multiq qdisc.
> > 

Oh well, this is a router.

So I think you need to change the device queue selection so that it only
depends on your filters / htb classes.

Otherwise flows for a particular 'customer' might be spread on the 8
queues, so the rate could be 8 * 1Mbit, instead of 1Mbit.

^ permalink raw reply

* Re: Using HTB over MultiQ
From: Eric Dumazet @ 2013-11-07 14:11 UTC (permalink / raw)
  To: Anton 'EvilMan' Danilov; +Cc: netdev
In-Reply-To: <CAEzD07+Haf-Kg52H7GsxC5XiQn2KXozHbxgL4Lvn0grhFXVFmA@mail.gmail.com>

On Thu, 2013-11-07 at 17:12 +0400, Anton 'EvilMan' Danilov wrote:
> Hello.
> 
> I'm experimenting with high performance linux router with 10G NICs.
> On high traffic rates the performance are limited by the lock of root
> queue discipline. For avoid impact of locking i've decided to build
> QoS scheme over the multiq qdisc.
> 
> And I have the issues with use to multiq discipline.
> 
> My setup:
> 1. Multiq qdisc is on top of interface.
> 2. To every multiq class i've attached htb discipline with own
> hierachy of child classes.
> 3. The filters (u32 with hashing) are attached to the root multiq discipline.
> 
> Graphical scheme of hierarchy -
> http://pixpin.ru/images/2013/11/07/multiq-hierarchy1.png
> 
> Fragments of script:
> 
> #add top qdisc and classes
>  qdisc add dev eth0 root handle 10: multiq
>  qdisc add dev eth0 parent 10:1 handle 11: htb
>  class add dev eth0 parent 11: classid 11:1 htb rate 1250Mbit
>  qdisc add dev eth0 parent 10:2 handle 12: htb
>  class add dev eth0 parent 12: classid 12:1 htb rate 1250Mbit
>  qdisc add dev eth0 parent 10:3 handle 13: htb
>  class add dev eth0 parent 13: classid 13:1 htb rate 1250Mbit
>  qdisc add dev eth0 parent 10:4 handle 14: htb
>  class add dev eth0 parent 14: classid 14:1 htb rate 1250Mbit
>  qdisc add dev eth0 parent 10:5 handle 15: htb
>  class add dev eth0 parent 15: classid 15:1 htb rate 1250Mbit
>  qdisc add dev eth0 parent 10:6 handle 16: htb
>  class add dev eth0 parent 16: classid 16:1 htb rate 1250Mbit
>  qdisc add dev eth0 parent 10:7 handle 17: htb
>  class add dev eth0 parent 17: classid 17:1 htb rate 1250Mbit
>  qdisc add dev eth0 parent 10:8 handle 18: htb
>  class add dev eth0 parent 18: classid 18:1 htb rate 1250Mbit
> 
> #add leaf classes and qdiscs (several hundreds)
>  ...
>  class add dev eth0 parent 11:1 classid 11:1736 htb rate 1024kbit
>  qdisc add dev eth0 parent 11:1736 handle 1736 pfifo limit 50
>  ...
> 
> But I see zero statistics on the leaf htb classes and nonzero
> statistics on the classifier filters:
> 
> ~$ tc -s -p filter list dev eth1
>  ...
>  filter parent 10: protocol ip pref 5 u32 fh 2:f2:800 order 2048 key
> ht 2 bkt f2 flowid 11:1736  (rule hit 306 success 306)
>    match IP src xx.xx.xx.xx/30 (success 306 )
>  ...
> 
> ~$ tc -s -s -d c ls dev eth1 classid 11:1736
>  class htb 11:1736 parent 11:1 leaf 1736: prio 0 quantum 12800 rate
> 1024Kbit ceil 1024Kbit burst 1599b/1 mpu 0b overhead 0b cburst 1599b/1
> mpu 0b overhead 0b level 0
>   Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
>   rate 0bit 0pps backlog 0b 0p requeues 0
>   lended: 0 borrowed: 0 giants: 0
>   tokens: 195312 ctokens: 195312
> 
> I think I've lost from view the some aspects of settings.
> Has anyone setuped the like complex scheme over the multiq discipline?
> 

I think this is not going to work, because multiqueue selection happens
before applying the filters to find a flowid.

And queue selection selects the queue depending on factors that are not
coupled to your filters, like cpu number

It looks like you want to rate limit a large number of flows, you could
try the following setup :

for ETH in eth0
do
 tc qd del dev $ETH root 2>/dev/null

 tc qd add dev $ETH root handle 100: mq 
 for i in `seq 1 8`
 do
  tc qd add dev $ETH parent 100:$i handle $i fq maxrate 1Mbit
 done
done

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox