Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next 1/5] sctp: remove the duplicate initialize
From: Wang Weidong @ 2013-11-08  2:55 UTC (permalink / raw)
  To: vyasevich, nhorman; +Cc: dingtianhong, davem, linux-sctp, netdev
In-Reply-To: <1383879310-22792-1-git-send-email-wangweidong1@huawei.com>

kzalloc had initialize the allocated memroy. Therefore, remove the
initialize with 0 and the memset.

Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
---
 net/sctp/associola.c | 41 -----------------------------------------
 1 file changed, 41 deletions(-)

diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index cef5099..667f984 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -90,14 +90,12 @@ static struct sctp_association *sctp_association_init(struct sctp_association *a
 
 	/* Initialize the object handling fields.  */
 	atomic_set(&asoc->base.refcnt, 1);
-	asoc->base.dead = false;
 
 	/* Initialize the bind addr area.  */
 	sctp_bind_addr_init(&asoc->base.bind_addr, ep->base.bind_addr.port);
 
 	asoc->state = SCTP_STATE_CLOSED;
 	asoc->cookie_life = ms_to_ktime(sp->assocparams.sasoc_cookie_life);
-	asoc->frag_point = 0;
 	asoc->user_frag = sp->user_frag;
 
 	/* Set the association max_retrans and RTO values from the
@@ -110,8 +108,6 @@ static struct sctp_association *sctp_association_init(struct sctp_association *a
 	asoc->rto_max = msecs_to_jiffies(sp->rtoinfo.srto_max);
 	asoc->rto_min = msecs_to_jiffies(sp->rtoinfo.srto_min);
 
-	asoc->overall_error_count = 0;
-
 	/* Initialize the association's heartbeat interval based on the
 	 * sock configured value.
 	 */
@@ -138,12 +134,9 @@ static struct sctp_association *sctp_association_init(struct sctp_association *a
 	asoc->max_burst = sp->max_burst;
 
 	/* initialize association timers */
-	asoc->timeouts[SCTP_EVENT_TIMEOUT_NONE] = 0;
 	asoc->timeouts[SCTP_EVENT_TIMEOUT_T1_COOKIE] = asoc->rto_initial;
 	asoc->timeouts[SCTP_EVENT_TIMEOUT_T1_INIT] = asoc->rto_initial;
 	asoc->timeouts[SCTP_EVENT_TIMEOUT_T2_SHUTDOWN] = asoc->rto_initial;
-	asoc->timeouts[SCTP_EVENT_TIMEOUT_T3_RTX] = 0;
-	asoc->timeouts[SCTP_EVENT_TIMEOUT_T4_RTO] = 0;
 
 	/* sctpimpguide Section 2.12.2
 	 * If the 'T5-shutdown-guard' timer is used, it SHOULD be set to the
@@ -152,7 +145,6 @@ static struct sctp_association *sctp_association_init(struct sctp_association *a
 	asoc->timeouts[SCTP_EVENT_TIMEOUT_T5_SHUTDOWN_GUARD]
 		= 5 * asoc->rto_max;
 
-	asoc->timeouts[SCTP_EVENT_TIMEOUT_HEARTBEAT] = 0;
 	asoc->timeouts[SCTP_EVENT_TIMEOUT_SACK] = asoc->sackdelay;
 	asoc->timeouts[SCTP_EVENT_TIMEOUT_AUTOCLOSE] =
 		min_t(unsigned long, sp->autoclose, net->sctp.max_autoclose) * HZ;
@@ -173,11 +165,6 @@ static struct sctp_association *sctp_association_init(struct sctp_association *a
 	asoc->max_init_timeo =
 		 msecs_to_jiffies(sp->initmsg.sinit_max_init_timeo);
 
-	/* Allocate storage for the ssnmap after the inbound and outbound
-	 * streams have been negotiated during Init.
-	 */
-	asoc->ssnmap = NULL;
-
 	/* Set the local window size for receive.
 	 * This is also the rcvbuf space per association.
 	 * RFC 6 - A SCTP receiver MUST be able to receive a minimum of
@@ -190,25 +177,15 @@ static struct sctp_association *sctp_association_init(struct sctp_association *a
 
 	asoc->a_rwnd = asoc->rwnd;
 
-	asoc->rwnd_over = 0;
-	asoc->rwnd_press = 0;
-
 	/* Use my own max window until I learn something better.  */
 	asoc->peer.rwnd = SCTP_DEFAULT_MAXWINDOW;
 
-	/* Set the sndbuf size for transmit.  */
-	asoc->sndbuf_used = 0;
-
 	/* Initialize the receive memory counter */
 	atomic_set(&asoc->rmem_alloc, 0);
 
 	init_waitqueue_head(&asoc->wait);
 
 	asoc->c.my_vtag = sctp_generate_tag(ep);
-	asoc->peer.i.init_tag = 0;     /* INIT needs a vtag of 0. */
-	asoc->c.peer_vtag = 0;
-	asoc->c.my_ttag   = 0;
-	asoc->c.peer_ttag = 0;
 	asoc->c.my_port = ep->base.bind_addr.port;
 
 	asoc->c.initial_tsn = sctp_generate_tsn(ep);
@@ -219,7 +196,6 @@ static struct sctp_association *sctp_association_init(struct sctp_association *a
 	asoc->adv_peer_ack_point = asoc->ctsn_ack_point;
 	asoc->highest_sacked = asoc->ctsn_ack_point;
 	asoc->last_cwr_tsn = asoc->ctsn_ack_point;
-	asoc->unack_data = 0;
 
 	/* ADDIP Section 4.1 Asconf Chunk Procedures
 	 *
@@ -238,7 +214,6 @@ static struct sctp_association *sctp_association_init(struct sctp_association *a
 
 	/* Make an empty list of remote transport addresses.  */
 	INIT_LIST_HEAD(&asoc->peer.transport_addr_list);
-	asoc->peer.transport_count = 0;
 
 	/* RFC 2960 5.1 Normal Establishment of an Association
 	 *
@@ -252,7 +227,6 @@ static struct sctp_association *sctp_association_init(struct sctp_association *a
 	 * already received one packet.]
 	 */
 	asoc->peer.sack_needed = 1;
-	asoc->peer.sack_cnt = 0;
 	asoc->peer.sack_generation = 1;
 
 	/* Assume that the peer will tell us if he recognizes ASCONF
@@ -260,12 +234,8 @@ static struct sctp_association *sctp_association_init(struct sctp_association *a
 	 * The sctp_addip_noauth option is there for backward compatibilty
 	 * and will revert old behavior.
 	 */
-	asoc->peer.asconf_capable = 0;
 	if (net->sctp.addip_noauth)
 		asoc->peer.asconf_capable = 1;
-	asoc->asconf_addr_del_pending = NULL;
-	asoc->src_out_of_asoc_ok = 0;
-	asoc->new_transport = NULL;
 
 	/* Create an input queue.  */
 	sctp_inq_init(&asoc->base.inqueue);
@@ -277,12 +247,6 @@ static struct sctp_association *sctp_association_init(struct sctp_association *a
 	if (!sctp_ulpq_init(&asoc->ulpq, asoc))
 		goto fail_init;
 
-	memset(&asoc->peer.tsn_map, 0, sizeof(struct sctp_tsnmap));
-
-	asoc->need_ecne = 0;
-
-	asoc->assoc_id = 0;
-
 	/* Assume that peer would support both address types unless we are
 	 * told otherwise.
 	 */
@@ -300,9 +264,6 @@ static struct sctp_association *sctp_association_init(struct sctp_association *a
 	asoc->default_timetolive = sp->default_timetolive;
 	asoc->default_rcv_context = sp->default_rcv_context;
 
-	/* SCTP_GET_ASSOC_STATS COUNTERS */
-	memset(&asoc->stats, 0, sizeof(struct sctp_priv_assoc_stats));
-
 	/* AUTH related initializations */
 	INIT_LIST_HEAD(&asoc->endpoint_shared_keys);
 	err = sctp_auth_asoc_copy_shkeys(ep, asoc, gfp);
@@ -310,9 +271,7 @@ static struct sctp_association *sctp_association_init(struct sctp_association *a
 		goto fail_init;
 
 	asoc->active_key_id = ep->active_key_id;
-	asoc->asoc_shared_key = NULL;
 
-	asoc->default_hmac_id = 0;
 	/* Save the hmacs and chunks list into this association */
 	if (ep->auth_hmacs_list)
 		memcpy(asoc->c.auth_hmacs, ep->auth_hmacs_list,
-- 
1.7.12

^ permalink raw reply related

* Re: [PATCH net-next 2/5] sctp: remove the else path
From: wangweidong @ 2013-11-08  3:07 UTC (permalink / raw)
  To: Joe Perches; +Cc: vyasevich, nhorman, dingtianhong, davem, linux-sctp, netdev
In-Reply-To: <1383879644.9263.62.camel@joe-AO722>

On 2013/11/8 11:00, Joe Perches wrote:
> On Fri, 2013-11-08 at 10:55 +0800, Wang Weidong wrote:
>> Make the code more simplification.
>>
>> Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
>> ---
>>  net/sctp/associola.c | 4 +---
>>  1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/net/sctp/associola.c b/net/sctp/associola.c
>> index 667f984..2d53d4c 100644
>> --- a/net/sctp/associola.c
>> +++ b/net/sctp/associola.c
>> @@ -954,15 +954,13 @@ int sctp_cmp_addr_exact(const union sctp_addr *ss1,
>>   */
>>  struct sctp_chunk *sctp_get_ecne_prepend(struct sctp_association *asoc)
>>  {
>> -	struct sctp_chunk *chunk;
>> +	struct sctp_chunk *chunk = NULL;
>>  
>>  	/* Send ECNE if needed.
>>  	 * Not being able to allocate a chunk here is not deadly.
>>  	 */
>>  	if (asoc->need_ecne)
>>  		chunk = sctp_make_ecne(asoc, asoc->last_ecne_tsn);
>> -	else
>> -		chunk = NULL;
>>  
>>  	return chunk;
>>  }
> 
> If you really want to make the code simple how about:
> 
> struct sctp_chunk *sctp_get_ecne_prepend(struct sctp_association *asoc)
> {
> 	if (!asoc->need_ecne)
> 		return NULL;
> 
> 	return sctp_make_ecne(asoc, asoc->last_ecne_tsn);
> }
> 
> 

Yeah, you are right. I will fix it in v2.
Thanks. 

> 
> 

^ permalink raw reply

* Re: [PATCH v4 net-next] net: introduce dev_set_forwarding()
From: Herbert Xu @ 2013-11-08  3:22 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David Miller, bhutchings, christoph.paasch, netdev, hkchu,
	mwdalton
In-Reply-To: <1383878554.9412.153.camel@edumazet-glaptop2.roam.corp.google.com>

On Thu, Nov 07, 2013 at 06:42:34PM -0800, Eric Dumazet wrote:
> On Fri, 2013-11-08 at 10:17 +0800, Herbert Xu wrote:
> 
> > Oh no let's keep it as it is indeed a great cleanup and improvement
> > for LRO.  We can however remove the bits that relate to GRO.
> 
> What about doing a benchmark first ?

If it really hurt then they can always turn it off using ethtool.
With your patch that automatically turns it off virt folks have
no way of even turning it on.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH v4 net-next] net: introduce dev_set_forwarding()
From: Herbert Xu @ 2013-11-08  3:23 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David Miller, bhutchings, christoph.paasch, netdev, hkchu,
	mwdalton
In-Reply-To: <1383879113.9412.155.camel@edumazet-glaptop2.roam.corp.google.com>

On Thu, Nov 07, 2013 at 06:51:53PM -0800, Eric Dumazet wrote:
> On Thu, 2013-11-07 at 18:42 -0800, Eric Dumazet wrote:
> 
> > A normal TSO packets with 16 MSS setups a ~17 DMA descriptors,
> > while GSO requires 2 DMA descriptors per MSS, plus a lot of overhead
> > in sk_buff allocation/deallocation.
> 
> Not mentioning fact that a 64KB packet is adding latencies, since high
> prio packets have to wait the whole preceding 64KB packet has left the
> host.

That would be a bug in the GRO code since a high prio packet
shouldn't have been merged in the first place and therefore
the usual priority mechanism should allow it to preempt the
64KB packet.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [net-next PATCH v4 0/2] l2 hardware accelerated macvlans
From: John Fastabend @ 2013-11-08  3:31 UTC (permalink / raw)
  To: David Miller
  Cc: nhorman, alexander.h.duyck, andy, netdev, jeffrey.t.kirsher,
	vyasevich
In-Reply-To: <20131107.191244.1462002095786588675.davem@davemloft.net>

On 11/7/2013 4:12 PM, David Miller wrote:
> From: John Fastabend <john.r.fastabend@intel.com>
> Date: Wed, 06 Nov 2013 09:54:40 -0800
>
>> This patch adds support to offload macvlan net_devices to the
>> hardware. With these patches packets are pushed to the macvlan
>> net_device directly and do not pass through the lower dev.
>
> Series applied, looks great.
>
> Thanks.
>

Thanks Dave. kbuild test robot reminds me that real_num_rx_queues is
only around with CONFIG_RPS. I'll get you a fix tonight.

.John

^ permalink raw reply

* Re: [PATCH net-next 3/5] sctp: if we done update ssnmap, free the new ssnmap
From: Vlad Yasevich @ 2013-11-08  3:49 UTC (permalink / raw)
  To: Wang Weidong, nhorman; +Cc: dingtianhong, davem, linux-sctp, netdev
In-Reply-To: <1383879310-22792-4-git-send-email-wangweidong1@huawei.com>

On 11/07/2013 09:55 PM, Wang Weidong wrote:
> I found that free the old parameters after update the peer parameters.
> So we should free the new ssnmap which not updated.
>
> Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
> ---
>   net/sctp/associola.c | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/net/sctp/associola.c b/net/sctp/associola.c
> index 2d53d4c..bca1ad1 100644
> --- a/net/sctp/associola.c
> +++ b/net/sctp/associola.c
> @@ -1223,6 +1223,10 @@ void sctp_assoc_update(struct sctp_association *asoc,
>   		}
>   	}
>
> +	/* if we don't update the ssnmap, we should free it. */
> +	if (new->ssnmap)
> +		sctp_ssnmap_free(new->ssnmap);
> +
>   	/* SCTP-AUTH: Save the peer parameters from the new assocaitions
>   	 * and also move the association shared keys over
>   	 */
>

Not necessary.  This will be done when the 'new' association is destroyed.

-vlad

^ permalink raw reply

* [PATCH] iproute2: Document type vlan option in ip-link(8)
From: Petr Šabata @ 2013-11-08  3:54 UTC (permalink / raw)
  To: stephen; +Cc: netdev, Petr Šabata

This patch adds the VLAN Type support section to the ip-link
manual page.

Signed-off-by: Petr Šabata <contyk@redhat.com>
---
 man/man8/ip-link.8.in | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/man/man8/ip-link.8.in b/man/man8/ip-link.8.in
index 8b68c78..4eddbcd 100644
--- a/man/man8/ip-link.8.in
+++ b/man/man8/ip-link.8.in
@@ -213,6 +213,65 @@ specifies the number of transmit queues for new device.
 specifies the number of receive queues for new device.
 
 .TP
+VLAN Type Support
+For a link of type
+.I VLAN
+the following additional arguments are supported:
+
+.BI "ip link add link " DEVICE
+.BI name " NAME "
+.BI type " vlan "
+.R " [ "
+.BI protocol " VLAN_PROTO "
+.R " ] "
+.BI id " VLANID "
+.R " [ "
+.BR reorder_hdr " { " on " | " off " } "
+.R " ] "
+.R " [ "
+.BR gvrp " { " on " | " off " } "
+.R " ] "
+.R " [ "
+.BR mvrp " { " on " | " off " } "
+.R " ] "
+.R " [ "
+.BR loose_binding " { " on " | " off " } "
+.R " ] "
+.R " [ "
+.BI ingress-qos-map " QOS-MAP "
+.R " ] "
+.R " [ "
+.BI egress-qos-map " QOS-MAP "
+.R " ] "
+
+.in +8
+.sp
+.BI protocol " VLAN_PROTO "
+- either 802.1Q or 802.1ad.
+
+.BI id " VLANID "
+- specifies the VID.
+
+.BR reorder_hdr " { " on " | " off " } "
+- specifies whether ethernet headers are reordered or not.
+
+.BR gvrp " { " on " | " off " } "
+- specifies whether this VLAN should be registered using GARP VLAN Registration Protocol.
+
+.BR mvrp " { " on " | " off " } "
+- specifies whether this VLAN should be registered using Multiple VLAN Registration Protocol.
+
+.BR loose_binding " { " on " | " off " } "
+- specifies whether the VLAN device state is bound to the physical device state.
+
+.BI ingress-qos-map " QOS-MAP "
+- defines a mapping between priority code points on incoming frames.  The format is FROM:TO with multiple mappings separated by spaces.
+
+.BI egress-qos-map " QOS-MAP "
+- the same as ingress-qos-map but for outgoing frames.
+.in -8
+
+.TP
 VXLAN Type Support
 For a link of type 
 .I VXLAN
-- 
1.8.3.1

^ permalink raw reply related

* Re: [PATCH v4 net-next] net: introduce dev_set_forwarding()
From: Herbert Xu @ 2013-11-08  3:59 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David Miller, bhutchings, christoph.paasch, netdev, hkchu,
	mwdalton
In-Reply-To: <1383861288.9412.89.camel@edumazet-glaptop2.roam.corp.google.com>

On Thu, Nov 07, 2013 at 01:54:48PM -0800, Eric Dumazet wrote:
>
> Hi Herbert
> 
> I believe I did this on my patch.

I actually quite like your patch for the pure iterative approach.

> Note that there is absolutely no requirement on how are the skb found in
> frag_list (their length is not a multiple of MSS)

OK I am thick and now I finally get what you're saying: virtio_net
as it stands does not produce the original GRO packet.  So my patch
is still broken with respect to virtio_net.  It won't crash anymore
but it'll just drop GRO/GSO packets which is probably worse.

> For the ease of discussion, once patched skb_segment() looks like :

However, I still have one reason for preferring my patch, it'll
be easier to prodce TSO packets with it.  Let me see if I can
fix up the arbitrary frag boundary issue without making it too
ugly.

Thanks!
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* [PATCH net-next v4 1/2] net_sched: fix some checkpatch errors
From: Yang Yingliang @ 2013-11-08  4:02 UTC (permalink / raw)
  To: davem, netdev; +Cc: stephen, dborkman, sergei.shtylyov
In-Reply-To: <1383883347-9320-1-git-send-email-yangyingliang@huawei.com>

There are some checkpatch errors, fix them.

Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
Suggested-by: Stephen Hemminger <stephen@networkplumber.org>
Suggested-by: Daniel Borkmann <dborkman@redhat.com>
Suggested-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
---
 net/sched/act_api.c     |  3 ++-
 net/sched/cls_bpf.c     |  2 +-
 net/sched/cls_u32.c     |  2 +-
 net/sched/sch_cbq.c     |  3 ++-
 net/sched/sch_generic.c |  4 ++--
 net/sched/sch_htb.c     |  9 +++++----
 net/sched/sch_netem.c   |  2 +-
 net/sched/sch_sfq.c     | 10 ++++++----
 8 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index fd70728..92a3c31 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 val;
 }
 EXPORT_SYMBOL(tcf_hash_new_index);
 
diff --git a/net/sched/cls_bpf.c b/net/sched/cls_bpf.c
index 1002a82..d7c72be 100644
--- a/net/sched/cls_bpf.c
+++ b/net/sched/cls_bpf.c
@@ -323,7 +323,7 @@ static int cls_bpf_dump(struct tcf_proto *tp, unsigned long fh,
 	if (nla == NULL)
 		goto nla_put_failure;
 
-        memcpy(nla_data(nla), prog->bpf_ops, nla_len(nla));
+	memcpy(nla_data(nla), prog->bpf_ops, nla_len(nla));
 
 	if (tcf_exts_dump(skb, &prog->exts, &bpf_ext_map) < 0)
 		goto nla_put_failure;
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index eb07a1e..59e546c 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -352,7 +352,7 @@ static int u32_destroy_key(struct tcf_proto *tp, struct tc_u_knode *n)
 	return 0;
 }
 
-static int u32_delete_key(struct tcf_proto *tp, struct tc_u_knode* key)
+static int u32_delete_key(struct tcf_proto *tp, struct tc_u_knode *key)
 {
 	struct tc_u_knode **kp;
 	struct tc_u_hnode *ht = key->ht_up;
diff --git a/net/sched/sch_cbq.c b/net/sched/sch_cbq.c
index 7a42c81..d5a8a4b 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) {
 				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;
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 7fc899a..9421350 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -338,13 +338,13 @@ EXPORT_SYMBOL(netif_carrier_off);
    cheaper.
  */
 
-static int noop_enqueue(struct sk_buff *skb, struct Qdisc * qdisc)
+static int noop_enqueue(struct sk_buff *skb, struct Qdisc *qdisc)
 {
 	kfree_skb(skb);
 	return NET_XMIT_CN;
 }
 
-static struct sk_buff *noop_dequeue(struct Qdisc * qdisc)
+static struct sk_buff *noop_dequeue(struct Qdisc *qdisc)
 {
 	return NULL;
 }
diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
index 0e1e38b..86c45a5 100644
--- a/net/sched/sch_htb.c
+++ b/net/sched/sch_htb.c
@@ -846,7 +846,7 @@ next:
 			break;
 
 		qdisc_warn_nonwc("htb", cl->un.leaf.q);
-		htb_next_rb_node(level ? &cl->parent->un.inner.clprio[prio].ptr:
+		htb_next_rb_node(level ? &cl->parent->un.inner.clprio[prio].ptr :
 					 &q->hlevel[0].hprio[prio].ptr);
 		cl = htb_lookup_leaf(hprio, prio);
 
@@ -1276,9 +1276,10 @@ static int htb_delete(struct Qdisc *sch, unsigned long arg)
 	struct Qdisc *new_q = NULL;
 	int last_child = 0;
 
-	// TODO: why don't allow to delete subtree ? references ? does
-	// tc subsys quarantee us that in htb_destroy it holds no class
-	// refs so that we can remove children safely there ?
+	/* TODO: why don't allow to delete subtree ? references ? does
+	 * tc subsys quarantee us that in htb_destroy it holds no class
+	 * refs so that we can remove children safely there ?
+	 */
 	if (cl->children || cl->filter_cnt)
 		return -EBUSY;
 
diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
index 75c94e5..9765c3f 100644
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -728,7 +728,7 @@ static int get_loss_clg(struct Qdisc *sch, const struct nlattr *attr)
 	nla_for_each_nested(la, attr, rem) {
 		u16 type = nla_type(la);
 
-		switch(type) {
+		switch (type) {
 		case NETEM_LOSS_GI: {
 			const struct tc_netem_gimodel *gi = nla_data(la);
 
diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
index d3a1bc2..76f01e0 100644
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -237,10 +237,12 @@ static inline void sfq_link(struct sfq_sched_data *q, sfq_index x)
 }
 
 #define sfq_unlink(q, x, n, p)			\
-	n = q->slots[x].dep.next;		\
-	p = q->slots[x].dep.prev;		\
-	sfq_dep_head(q, p)->next = n;		\
-	sfq_dep_head(q, n)->prev = p
+	do {					\
+		n = q->slots[x].dep.next;	\
+		p = q->slots[x].dep.prev;	\
+		sfq_dep_head(q, p)->next = n;	\
+		sfq_dep_head(q, n)->prev = p;	\
+	} while (0)
 
 
 static inline void sfq_dec(struct sfq_sched_data *q, sfq_index x)
-- 
1.7.12

^ permalink raw reply related

* [PATCH net-next v4 0/2] net_sched: Fix some checkpatch errors and use pr_warn
From: Yang Yingliang @ 2013-11-08  4:02 UTC (permalink / raw)
  To: davem, netdev; +Cc: stephen, dborkman, sergei.shtylyov

Fix some checkpatch errors and
replace pr_warning with pr_warn.

v1 -> v2:
  - patch 1/2: return an assignment on seperate line
    in tcf_hash_new_index() suggested by Stephen.

v2 -> v3:
  - patch 1/2: don't remove global vars' initialization.
  - patch 2/2: adjust pr_warn content make more comfortable.

v3 -> v4:
  - patch 1/2: do correct indent.
  - patch 2/2: do correct indent and recover pr_debug to printk.

Yang Yingliang (2):
  net_sched: fix some checkpatch errors
  net_sched: Use pr_warn replace pr_warning

 net/sched/act_api.c     |  3 ++-
 net/sched/cls_bpf.c     |  2 +-
 net/sched/cls_u32.c     |  2 +-
 net/sched/sch_cbq.c     |  7 ++++---
 net/sched/sch_dsmark.c  |  4 ++--
 net/sched/sch_generic.c |  4 ++--
 net/sched/sch_gred.c    |  4 ++--
 net/sched/sch_htb.c     | 23 ++++++++++++-----------
 net/sched/sch_netem.c   |  2 +-
 net/sched/sch_sfq.c     | 10 ++++++----
 10 files changed, 33 insertions(+), 28 deletions(-)

-- 
1.7.12

^ permalink raw reply

* [PATCH net-next v4 2/2] net_sched: Use pr_warn replace pr_warning
From: Yang Yingliang @ 2013-11-08  4:02 UTC (permalink / raw)
  To: davem, netdev; +Cc: stephen, dborkman, sergei.shtylyov
In-Reply-To: <1383883347-9320-1-git-send-email-yangyingliang@huawei.com>

Replace pr_warning() with pr_warn().

Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
Suggested-by: Daniel Borkmann <dborkman@redhat.com>
Suggested-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
---
 net/sched/sch_cbq.c    |  4 ++--
 net/sched/sch_dsmark.c |  4 ++--
 net/sched/sch_gred.c   |  4 ++--
 net/sched/sch_htb.c    | 14 +++++++-------
 4 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/net/sched/sch_cbq.c b/net/sched/sch_cbq.c
index d5a8a4b..ff46f60 100644
--- a/net/sched/sch_cbq.c
+++ b/net/sched/sch_cbq.c
@@ -1060,8 +1060,8 @@ 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",
-					   cl->common.classid, cl->quantum);
+				pr_warn("CBQ: class %08x has bad quantum==%ld, repaired.\n",
+					cl->common.classid, cl->quantum);
 				cl->quantum = qdisc_dev(cl->qdisc)->mtu/2 + 1;
 			}
 		}
diff --git a/net/sched/sch_dsmark.c b/net/sched/sch_dsmark.c
index 3886365..097ac66 100644
--- a/net/sched/sch_dsmark.c
+++ b/net/sched/sch_dsmark.c
@@ -303,8 +303,8 @@ static struct sk_buff *dsmark_dequeue(struct Qdisc *sch)
 		 * and don't need yet another qdisc as a bypass.
 		 */
 		if (p->mask[index] != 0xff || p->value[index])
-			pr_warning("dsmark_dequeue: unsupported protocol %d\n",
-				   ntohs(skb->protocol));
+			pr_warn("dsmark_dequeue: unsupported protocol %d\n",
+				ntohs(skb->protocol));
 		break;
 	}
 
diff --git a/net/sched/sch_gred.c b/net/sched/sch_gred.c
index d42234c..fb621d2 100644
--- a/net/sched/sch_gred.c
+++ b/net/sched/sch_gred.c
@@ -370,8 +370,8 @@ static inline int gred_change_table_def(struct Qdisc *sch, struct nlattr *dps)
 
 	for (i = table->DPs; i < MAX_DPs; i++) {
 		if (table->tab[i]) {
-			pr_warning("GRED: Warning: Destroying "
-				   "shadowed VQ 0x%x\n", i);
+			pr_warn("GRED: Warning: Destroying "
+				"shadowed VQ 0x%x\n", i);
 			gred_destroy_vq(table->tab[i]);
 			table->tab[i] = NULL;
 		}
diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
index 86c45a5..0ed048d 100644
--- a/net/sched/sch_htb.c
+++ b/net/sched/sch_htb.c
@@ -712,7 +712,7 @@ static s64 htb_do_events(struct htb_sched *q, const int level,
 
 	/* too much load - let's continue after a break for scheduling */
 	if (!(q->warned & HTB_WARN_TOOMANYEVENTS)) {
-		pr_warning("htb: too many events!\n");
+		pr_warn("htb: too many events!\n");
 		q->warned |= HTB_WARN_TOOMANYEVENTS;
 	}
 
@@ -1484,15 +1484,15 @@ 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(
-			       "HTB: quantum of class %X is small. Consider r2q change.\n",
-			       cl->common.classid);
+			pr_warn("HTB: quantum of class %X is small. "
+				"Consider r2q change.\n",
+				cl->common.classid);
 			cl->quantum = 1000;
 		}
 		if (!hopt->quantum && cl->quantum > 200000) {
-			pr_warning(
-			       "HTB: quantum of class %X is big. Consider r2q change.\n",
-			       cl->common.classid);
+			pr_warn("HTB: quantum of class %X is big. "
+				"Consider r2q change.\n",
+				cl->common.classid);
 			cl->quantum = 200000;
 		}
 		if (hopt->quantum)
-- 
1.7.12

^ permalink raw reply related

* Re: [PATCH v4 net-next] net: introduce dev_set_forwarding()
From: Eric Dumazet @ 2013-11-08  4:06 UTC (permalink / raw)
  To: Herbert Xu
  Cc: David Miller, bhutchings, christoph.paasch, netdev, hkchu,
	mwdalton
In-Reply-To: <20131108032208.GA14056@gondor.apana.org.au>

On Fri, 2013-11-08 at 11:22 +0800, Herbert Xu wrote:

> If it really hurt then they can always turn it off using ethtool.
> With your patch that automatically turns it off virt folks have
> no way of even turning it on.

Let me remind you that before the GRO frag_list patch, virt folks had no
choice anyway. GRO packets were limited to 16 MSS.

Its not because we can build large packets that we must do so.

This was an error of our TSO implementation.

^ permalink raw reply

* Re: [PATCH v4 net-next] net: introduce dev_set_forwarding()
From: Herbert Xu @ 2013-11-08  4:10 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David Miller, bhutchings, christoph.paasch, netdev, hkchu,
	mwdalton
In-Reply-To: <1383883594.9412.158.camel@edumazet-glaptop2.roam.corp.google.com>

On Thu, Nov 07, 2013 at 08:06:34PM -0800, Eric Dumazet wrote:
> On Fri, 2013-11-08 at 11:22 +0800, Herbert Xu wrote:
> 
> > If it really hurt then they can always turn it off using ethtool.
> > With your patch that automatically turns it off virt folks have
> > no way of even turning it on.
> 
> Let me remind you that before the GRO frag_list patch, virt folks had no
> choice anyway. GRO packets were limited to 16 MSS.
> 
> Its not because we can build large packets that we must do so.
> 
> This was an error of our TSO implementation.

Well you still don't seem to be getting this: If you need it for
the host then you will need it even more for virt because the
network stack there is much longer.

So having it only available to the host without also giving it
to virt makes *zero* sense.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [net-next PATCH v4 0/2] l2 hardware accelerated macvlans
From: Vlad Yasevich @ 2013-11-08  4:18 UTC (permalink / raw)
  To: John Fastabend, nhorman, davem
  Cc: alexander.h.duyck, andy, netdev, jeffrey.t.kirsher, vyasevich
In-Reply-To: <20131106175308.3822.73239.stgit@jf-dev1-dcblab>

On 11/06/2013 12:54 PM, John Fastabend wrote:
> This patch adds support to offload macvlan net_devices to the
> hardware. With these patches packets are pushed to the macvlan
> net_device directly and do not pass through the lower dev.
>
> The patches here have made it through multiple iterations
> each with a slightly different focus. First I tried to
> push these as a new link type called "VMDQ". The patches
> shown here,
>
> http://comments.gmane.org/gmane.linux.network/237617
>
> Following this implementation I renamed the link type
> "VSI" and addressed various comments. Finally Neil
> Horman picked up the patches and integrated the offload
> into the macvlan code. Here,
>
> http://permalink.gmane.org/gmane.linux.network/285658
>
> The attached series is clean-up of his patches, with a
> few fixes.
>
> If folks find this series acceptable there are a few
> items we can work on next. First broadcast and multicast
> will use the hardware even for local traffic with this
> series. It would be best (I think) to use the software
> path for macvlan to macvlan traffic and save the PCIe
> bus. This depends on how much you value CPU time vs
> PCIE bandwidth. This will need another patch series
> to flush out.
>

John

So, I've been looking at these patches and the more I
look the more I wonder how much benefit there is in
sending unicast macvlan<->macvlan traffic through the hw.
It looks like any bulk transfers would have to undergo
tso segmentaion and GRO, where as this can be completely
bypassed right now with software.

Did you run any numbers?

Thanks
-vlad


> Also this series only allows for layer 2 mac forwarding
> where some hardware supports more interesting forwarding
> capabilities. Integrating with OVS may be useful here.
>
> As always any comments/feedback welcome.
>
> My basic I/O test is here but I've also done some link
> testing, SRIOV/DCB with macvlans and others,
>
> #ip link add link eth2 numtxqueues 4 numrxqueues 4 txqueuelen 50 type macvlan
> #tc qdisc add dev macvlan0 mq
> #iperf -c 10.0.0.1 -P 8 -t 5000 -i 10
>
> Changelog:
> v2: two fixes to ixgbe when all features DCB, FCoE, SR-IOV
>      are enabled with macvlans. A VMDQ_P() reference
>      should have been accel->pool and do not set the offset
>      of the ring index from dfwd add call. The offset is used
>      by SR-IOV so clearing it can cause SR-IOV quue index's
>      to go sideways. With these fixes testing macvlan's with
>      SRIOV enabled was successful.
> v3: addressed Neil's comments in ixgbe
>      fixed error path on dfwd_add_station() in ixgbe
>      fixed ixgbe to allow SRIOV and accelerated macvlans to
>      coexist.
> v4: Dave caught some strange indentation, fixed it here
> ---
>
> John Fastabend (2):
>        ixgbe: enable l2 forwarding acceleration for macvlans
>        net: Add layer 2 hardware acceleration operations for macvlan devices
>
>
>   drivers/net/ethernet/intel/ixgbe/ixgbe.h       |   20 +
>   drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c   |   15 +
>   drivers/net/ethernet/intel/ixgbe/ixgbe_main.c  |  480 ++++++++++++++++++++----
>   drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c |   17 -
>   drivers/net/macvlan.c                          |   36 ++
>   include/linux/if_macvlan.h                     |    1
>   include/linux/netdev_features.h                |    2
>   include/linux/netdevice.h                      |   36 ++
>   include/uapi/linux/if.h                        |    1
>   net/core/dev.c                                 |   18 +
>   net/core/ethtool.c                             |    1
>   net/sched/sch_generic.c                        |    2
>   12 files changed, 532 insertions(+), 97 deletions(-)
>

^ permalink raw reply

* Re: [PATCH v4 net-next] net: introduce dev_set_forwarding()
From: Eric Dumazet @ 2013-11-08  4:21 UTC (permalink / raw)
  To: Herbert Xu
  Cc: David Miller, bhutchings, christoph.paasch, netdev, hkchu,
	mwdalton
In-Reply-To: <20131108032359.GA14132@gondor.apana.org.au>

On Fri, 2013-11-08 at 11:23 +0800, Herbert Xu wrote:
> On Thu, Nov 07, 2013 at 06:51:53PM -0800, Eric Dumazet wrote:
> > On Thu, 2013-11-07 at 18:42 -0800, Eric Dumazet wrote:
> > 
> > > A normal TSO packets with 16 MSS setups a ~17 DMA descriptors,
> > > while GSO requires 2 DMA descriptors per MSS, plus a lot of overhead
> > > in sk_buff allocation/deallocation.
> > 
> > Not mentioning fact that a 64KB packet is adding latencies, since high
> > prio packets have to wait the whole preceding 64KB packet has left the
> > host.
> 
> That would be a bug in the GRO code since a high prio packet
> shouldn't have been merged in the first place and therefore
> the usual priority mechanism should allow it to preempt the
> 64KB packet.

Some users install Qdisc (AQM) on their router, to decide of what is
high priority and what is not. Their iptables or qdisc filters can be
quite complex.

Its all TCP for example.

GRO stack cannot make this decision.

So lets say we receive on ingress a mix of high prio packets and low
prio TCP packets. If GRO stack is able to build super big GRO packet,
then this super big GRO packet is a head of line blocking.

At 1Gbps, a 16 MSS packet is holding the line for about 190 us.

At 45 MSS, you basically multiply by 3 this latency.

What we probably want is a way to tune this latency, not ignore the
problem by making big GRO packets.

The only current choice for the user is to enable or disable GRO per
ingress port.

Thats a trivial patch, but net-next is closed at this moment.

^ permalink raw reply

* Re: [PATCH v4 net-next] net: introduce dev_set_forwarding()
From: Herbert Xu @ 2013-11-08  4:24 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David Miller, bhutchings, christoph.paasch, netdev, hkchu,
	mwdalton
In-Reply-To: <1383884517.9412.172.camel@edumazet-glaptop2.roam.corp.google.com>

On Thu, Nov 07, 2013 at 08:21:57PM -0800, Eric Dumazet wrote:
> On Fri, 2013-11-08 at 11:23 +0800, Herbert Xu wrote:
> > On Thu, Nov 07, 2013 at 06:51:53PM -0800, Eric Dumazet wrote:
> > > On Thu, 2013-11-07 at 18:42 -0800, Eric Dumazet wrote:
> > > 
> > > > A normal TSO packets with 16 MSS setups a ~17 DMA descriptors,
> > > > while GSO requires 2 DMA descriptors per MSS, plus a lot of overhead
> > > > in sk_buff allocation/deallocation.
> > > 
> > > Not mentioning fact that a 64KB packet is adding latencies, since high
> > > prio packets have to wait the whole preceding 64KB packet has left the
> > > host.
> > 
> > That would be a bug in the GRO code since a high prio packet
> > shouldn't have been merged in the first place and therefore
> > the usual priority mechanism should allow it to preempt the
> > 64KB packet.
> 
> Some users install Qdisc (AQM) on their router, to decide of what is
> high priority and what is not. Their iptables or qdisc filters can be
> quite complex.
> 
> Its all TCP for example.
> 
> GRO stack cannot make this decision.
> 
> So lets say we receive on ingress a mix of high prio packets and low
> prio TCP packets. If GRO stack is able to build super big GRO packet,
> then this super big GRO packet is a head of line blocking.
> 
> At 1Gbps, a 16 MSS packet is holding the line for about 190 us.
> 
> At 45 MSS, you basically multiply by 3 this latency.
> 
> What we probably want is a way to tune this latency, not ignore the
> problem by making big GRO packets.
> 
> The only current choice for the user is to enable or disable GRO per
> ingress port.
> 
> Thats a trivial patch, but net-next is closed at this moment.

I don't know why you're fixing this problem by disabling/degrading
GRO instead of fixing the qdisc to intelligently segment the GRO
packet.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH v4 net-next] net: introduce dev_set_forwarding()
From: Eric Dumazet @ 2013-11-08  4:24 UTC (permalink / raw)
  To: Herbert Xu
  Cc: David Miller, bhutchings, christoph.paasch, netdev, hkchu,
	mwdalton
In-Reply-To: <20131108041016.GB14325@gondor.apana.org.au>

On Fri, 2013-11-08 at 12:10 +0800, Herbert Xu wrote:

> Well you still don't seem to be getting this: If you need it for
> the host then you will need it even more for virt because the
> network stack there is much longer.
> 
> So having it only available to the host without also giving it
> to virt makes *zero* sense.

I am fine with this, but how can we know the packet is going to be
delivered to virt instead of forwarded to ethernet ?

Do we want the equivalent of 'IP early demux', done at GRO layer ? ;)

^ permalink raw reply

* Re: [PATCH v4 net-next] net: introduce dev_set_forwarding()
From: Eric Dumazet @ 2013-11-08  4:25 UTC (permalink / raw)
  To: Herbert Xu
  Cc: David Miller, bhutchings, christoph.paasch, netdev, hkchu,
	mwdalton
In-Reply-To: <20131108035949.GA14325@gondor.apana.org.au>

On Fri, 2013-11-08 at 11:59 +0800, Herbert Xu wrote:

> However, I still have one reason for preferring my patch, it'll
> be easier to prodce TSO packets with it.  Let me see if I can
> fix up the arbitrary frag boundary issue without making it too
> ugly.

Sure !

^ permalink raw reply

* Re: [PATCH v4 net-next] net: introduce dev_set_forwarding()
From: Herbert Xu @ 2013-11-08  4:28 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David Miller, bhutchings, christoph.paasch, netdev, hkchu,
	mwdalton
In-Reply-To: <1383884683.9412.174.camel@edumazet-glaptop2.roam.corp.google.com>

On Thu, Nov 07, 2013 at 08:24:43PM -0800, Eric Dumazet wrote:
> On Fri, 2013-11-08 at 12:10 +0800, Herbert Xu wrote:
> 
> > Well you still don't seem to be getting this: If you need it for
> > the host then you will need it even more for virt because the
> > network stack there is much longer.
> > 
> > So having it only available to the host without also giving it
> > to virt makes *zero* sense.
> 
> I am fine with this, but how can we know the packet is going to be
> delivered to virt instead of forwarded to ethernet ?

That's the crux of our disagreement :)

My preference is that if we can make it work at least as good
as before (if not better) in the forwarding case then we won't
need to care.

And so far I haven't seen any convincing argument why this cannot
be fixed in the forwarding case.

> Do we want the equivalent of 'IP early demux', done at GRO layer ? ;)

Won't help because the virt guest may end up forwarding the packet
anyway and you can't rely on it telling you that.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH net-next 3/5] sctp: if we done update ssnmap, free the new ssnmap
From: wangweidong @ 2013-11-08  4:30 UTC (permalink / raw)
  To: Vlad Yasevich, nhorman; +Cc: dingtianhong, davem, linux-sctp, netdev
In-Reply-To: <527C5F3E.3090200@gmail.com>

On 2013/11/8 11:49, Vlad Yasevich wrote:
> On 11/07/2013 09:55 PM, Wang Weidong wrote:
>> I found that free the old parameters after update the peer parameters.
>> So we should free the new ssnmap which not updated.
>>
>> Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
>> ---
>>   net/sctp/associola.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/net/sctp/associola.c b/net/sctp/associola.c
>> index 2d53d4c..bca1ad1 100644
>> --- a/net/sctp/associola.c
>> +++ b/net/sctp/associola.c
>> @@ -1223,6 +1223,10 @@ void sctp_assoc_update(struct sctp_association *asoc,
>>           }
>>       }
>>
>> +    /* if we don't update the ssnmap, we should free it. */
>> +    if (new->ssnmap)
>> +        sctp_ssnmap_free(new->ssnmap);
>> +
>>       /* SCTP-AUTH: Save the peer parameters from the new assocaitions
>>        * and also move the association shared keys over
>>        */
>>
> 
> Not necessary.  This will be done when the 'new' association is destroyed.
> 
> -vlad
>

Yeah, it is not necessary done in sctp_assoc_update.
Thanks. 

> 

^ permalink raw reply

* Re: [PATCH v4 net-next] net: introduce dev_set_forwarding()
From: Eric Dumazet @ 2013-11-08  4:40 UTC (permalink / raw)
  To: Herbert Xu
  Cc: David Miller, bhutchings, christoph.paasch, netdev, hkchu,
	mwdalton
In-Reply-To: <20131108042425.GA14577@gondor.apana.org.au>

On Fri, 2013-11-08 at 12:24 +0800, Herbert Xu wrote:

> I don't know why you're fixing this problem by disabling/degrading
> GRO instead of fixing the qdisc to intelligently segment the GRO
> packet.

On what criteria you decide that the packet you are going to queue to
the NIC must be segmented ?

How do you predict the future ?

BTW, the Qdisc API doesnt really permit what you describe.

There are the queue() and dequeue(). Once packet is dequeued, you cannot
requeue a bunch of packets if you decide to segment the GRO packet.

This would require a lot of changes, while its much easier to give a
limit to GRO layer (replace the 65536 constant in skb_gro_receive() to
dev->max_gro_size).

Its rare the same host is used both as a latency sensitive router, and a
virt box...

^ permalink raw reply

* Re: [PATCH v4 net-next] net: introduce dev_set_forwarding()
From: Herbert Xu @ 2013-11-08  4:43 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David Miller, bhutchings, christoph.paasch, netdev, hkchu,
	mwdalton
In-Reply-To: <1383885600.9412.187.camel@edumazet-glaptop2.roam.corp.google.com>

On Thu, Nov 07, 2013 at 08:40:00PM -0800, Eric Dumazet wrote:
> On Fri, 2013-11-08 at 12:24 +0800, Herbert Xu wrote:
> 
> > I don't know why you're fixing this problem by disabling/degrading
> > GRO instead of fixing the qdisc to intelligently segment the GRO
> > packet.
> 
> On what criteria you decide that the packet you are going to queue to
> the NIC must be segmented ?

Easy.  You don't physically segment the packet until it's
transmitted.  But logically you treat each GRO/GSO packet as if it
had already been segmented.

> BTW, the Qdisc API doesnt really permit what you describe.
> 
> There are the queue() and dequeue(). Once packet is dequeued, you cannot
> requeue a bunch of packets if you decide to segment the GRO packet.

This logic should be inside the qdisc.  I'm not saying that we
need to implement this for every single qdisc out there, just the
ones that you care about.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* linux-next: manual merge of the tip tree with the net-next tree
From: Stephen Rothwell @ 2013-11-08  4:58 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Peter Zijlstra
  Cc: linux-next, linux-kernel, John Stultz, Jason Wang, David Miller,
	netdev

[-- Attachment #1: Type: text/plain, Size: 1195 bytes --]

Hi all,

Today's linux-next merge of the tip tree got a conflict in
drivers/net/virtio_net.c between commit 9bb8ca86075f ("virtio-net: switch
to use XPS to choose txq") from the net-next tree and commit 827da44c6141
("net: Explicitly initialize u64_stats_sync structures for lockdep") from
the tip tree.

I fixed it up (see below) and can carry the fix as necessary (no action
is required).

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

diff --cc drivers/net/virtio_net.c
index 7d0eff710913,ee384f3d612b..000000000000
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@@ -1584,6 -1569,18 +1584,14 @@@ static int virtnet_probe(struct virtio_
  	if (vi->stats == NULL)
  		goto free;
  
+ 	for_each_possible_cpu(i) {
+ 		struct virtnet_stats *virtnet_stats;
+ 		virtnet_stats = per_cpu_ptr(vi->stats, i);
+ 		u64_stats_init(&virtnet_stats->tx_syncp);
+ 		u64_stats_init(&virtnet_stats->rx_syncp);
+ 	}
+ 
+ 
 -	vi->vq_index = alloc_percpu(int);
 -	if (vi->vq_index == NULL)
 -		goto free_stats;
 -
  	mutex_init(&vi->config_lock);
  	vi->config_enable = true;
  	INIT_WORK(&vi->config_work, virtnet_config_changed_work);

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH v4 net-next] net: introduce dev_set_forwarding()
From: Eric Dumazet @ 2013-11-08  5:08 UTC (permalink / raw)
  To: Herbert Xu
  Cc: David Miller, bhutchings, christoph.paasch, netdev, hkchu,
	mwdalton
In-Reply-To: <20131108044335.GA14787@gondor.apana.org.au>

On Fri, 2013-11-08 at 12:43 +0800, Herbert Xu wrote:

> Easy.  You don't physically segment the packet until it's
> transmitted.  But logically you treat each GRO/GSO packet as if it
> had already been segmented.

OK, please submit some patches, because I must be blind.

^ permalink raw reply

* Re: [PATCH v4 net-next] net: introduce dev_set_forwarding()
From: Herbert Xu @ 2013-11-08  5:21 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David Miller, bhutchings, christoph.paasch, netdev, hkchu,
	mwdalton
In-Reply-To: <1383887329.9412.207.camel@edumazet-glaptop2.roam.corp.google.com>

On Thu, Nov 07, 2013 at 09:08:49PM -0800, Eric Dumazet wrote:
> On Fri, 2013-11-08 at 12:43 +0800, Herbert Xu wrote:
> 
> > Easy.  You don't physically segment the packet until it's
> > transmitted.  But logically you treat each GRO/GSO packet as if it
> > had already been segmented.
> 
> OK, please submit some patches, because I must be blind.

Honestly it doesn't even matter.  If it's really so bad for your
qdisc, you should just call skb_segment in your qdisc directly.

My point is that even if you did that GRO with your frag_list
patch should still be a win because the stack prior to the qdisc
gets run once instead of two or three times.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ 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