Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 04/11] hso: fix memory leak in hso_create_rfkill()
From: Olivier Sobrie @ 2015-01-20 15:10 UTC (permalink / raw)
  To: Oliver Neukum
  Cc: Jan Dumon, Greg Kroah-Hartman, linux-kernel, linux-usb, netdev
In-Reply-To: <1421759597.29486.22.camel@linux-0dmf.site>

On Tue, Jan 20, 2015 at 02:13:17PM +0100, Oliver Neukum wrote:
> On Tue, 2015-01-20 at 13:29 +0100, Olivier Sobrie wrote:
> > When the rfkill interface was created, a buffer containing the name
> > of the rfkill node was allocated. This buffer was never freed when the
> > device disappears.
> > 
> > To fix the problem, we put the name given to rfkill_alloc() in
> > the hso_net structure.
> > 
> > Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
> > ---
> >  drivers/net/usb/hso.c | 12 +++---------
> >  1 file changed, 3 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
> > index 470ef9e..a49ac2e 100644
> > --- a/drivers/net/usb/hso.c
> > +++ b/drivers/net/usb/hso.c
> > @@ -153,6 +153,7 @@ struct hso_net {
> >  	struct hso_device *parent;
> >  	struct net_device *net;
> >  	struct rfkill *rfkill;
> > +	char name[8];
> >  
> >  	struct usb_endpoint_descriptor *in_endp;
> >  	struct usb_endpoint_descriptor *out_endp;
> > @@ -2467,27 +2468,20 @@ static void hso_create_rfkill(struct hso_device *hso_dev,
> >  {
> >  	struct hso_net *hso_net = dev2net(hso_dev);
> >  	struct device *dev = &hso_net->net->dev;
> > -	char *rfkn;
> >  
> > -	rfkn = kzalloc(20, GFP_KERNEL);
> > -	if (!rfkn)
> > -		dev_err(dev, "%s - Out of memory\n", __func__);
> > -
> > -	snprintf(rfkn, 20, "hso-%d",
> > +	snprintf(hso_net->name, sizeof(hso_net->name), "hso-%d",
> >  		 interface->altsetting->desc.bInterfaceNumber);
> 
> That number is not unique. Indeed it will be identical for all devices.

Indeed. That should be corrected too.
Thank you,

Olivier

^ permalink raw reply

* Re: [PATCH 11/11] usb: core: fix a race with usb_queue_reset_device()
From: Olivier Sobrie @ 2015-01-20 15:10 UTC (permalink / raw)
  To: Oliver Neukum
  Cc: Jan Dumon, Greg Kroah-Hartman, linux-kernel, linux-usb, netdev
In-Reply-To: <1421761717.29486.24.camel@linux-0dmf.site>

Hi Oliver,

On Tue, Jan 20, 2015 at 02:48:37PM +0100, Oliver Neukum wrote:
> On Tue, 2015-01-20 at 13:29 +0100, Olivier Sobrie wrote:
> > When usb_queue_reset() is called it schedules a work in view of
> > resetting the usb interface. When the reset work is running, it
> > can be scheduled again (e.g. by the usb disconnect method of
> > the driver).
> > 
> > Consider that the reset work is queued again while the reset work
> > is running and that this work leads to a forced unbinding of the
> > usb interface (e.g. because a driver is bound to the interface
> > and has no pre/post_reset methods - see usb_reset_device()).
> > In such condition, usb_unbind_interface() gets called and this
> > function calls usb_cancel_queued_reset() which does nothing
> > because the flag "reset_running" is set to 1. The second reset
> > work that has been scheduled is therefore not cancelled.
> > Later, the usb_reset_device() tries to rebind the interface.
> > If it fails, then the usb interface context which contain the
> > reset work struct is freed and it most likely crash when the
> > second reset work tries to be run.
> > 
> > The following flow shows the problem:
> > * usb_queue_reset_device()
> > * __usb_queue_reset_device() <- If the reset work is queued after here, then
> >     reset_running = 1           it will never be cancelled.
> >     usb_reset_device()
> >       usb_forced_unbind_intf()
> >         usb_driver_release_interface()
> >           usb_unbind_interface()
> >             driver->disconnect()
> >               usb_queue_reset_device() <- second reset
> 
> That is the sledgehammer approach. Wouldn't it be better to guarantee
> that usb_queue_reset_device() be a nop when reset_running==1 ?

If I'm right, we have to prevent that usb_queue_reset_device() shedules
the work a second time before the variable reset_running is set.
An other task can requeue a reset while the work __usb_queue_reset_device()
is busy but when the flag reset_running hasn't been set yet.

I see different other approaches to solve the problem:

 * Setting a flag in the usb_queue_reset_device() when a reset has been
   scheduled and resetting this flag when the reset is done. This implies
   a locking mechanism around the flag.

 * Avoid that the hso driver queues multiple resets by using a flag. It
   also requires locking. It comes more or less to the same solution
   as the previous one but the patch is done in the hso driver.

 * using get_device() and put_device() to avoid that the usb interface
   structure get freed before the second reset is run.
   I mean:
	void usb_queue_reset_device(struct usb_interface *iface)
	{
		get_device()
		if (!schedule_work(&iface->reset_ws))
			put_device()
	}

	static void __usb_queue_reset_device(struct work_struct *ws)
	{
		...
		put_device()
	}

   But this solution does not avoid the second reset...

If you have other better ideas, let me know.
Correct me if I'm wrong.

Thank you,

Olivier

> 
> >             usb_cancel_queued_reset() <- does nothing because
> >                                          the flag reset_running
> >                                          is set
> >       usb_unbind_and_rebind_marked_interfaces()
> >         usb_rebind_intf()
> >           device_attach()
> >             driver->probe() <- fails (no more drivers hold a reference to
> > 				      the usb interface)
> >     reset_running = 0
> > * hub_event()
> >     usb_disconnect()
> >       usb_disable_device()
> >         kobject_release()
> >           device_release()
> >             usb_release_interface()
> >               kfree(intf) <- usb interface context is released
> >                              while we still have a pending reset
> >                              work that should be run
> > 
> > To avoid this problem, we use a delayed work so that if the reset
> > work is currently run, we can avoid further call to
> > __usb_queue_reset_device() work by using cancel_delayed_work().
> > Unfortunately it increases the size of the usb_interface structure...
> 
> 	Regards
> 		Oliver
> 
> -- 
> Oliver Neukum <oneukum@suse.de>
> 

-- 
Olivier

^ permalink raw reply

* Re: [PATCH 3/3] netlink: Lock out table resizes while dumping Netlink sockets
From: Thomas Graf @ 2015-01-20 15:05 UTC (permalink / raw)
  To: David Laight
  Cc: 'Patrick McHardy', davem@davemloft.net,
	herbert@gondor.apana.org.au, paulmck@linux.vnet.ibm.com,
	ying.xue@windriver.com, netdev@vger.kernel.org,
	netfilter-devel@vger.kernel.org
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D1CACF400@AcuExch.aculab.com>

On 01/20/15 at 03:00pm, David Laight wrote:
> From: Patrick McHardy
> > An alternative would be to set a flag in ht when a dump begins that
> > indicates to skip resizing operations and on the end of the dump
> > perform any resizing operations that might be necessary. Herbert
> > disagrees though and he might be right.
> 
> Can you rely on being told when the dump completes?
> If the program is killed in the middle then it can't tell you.
> 
> I suspect you'd have to suppress resize for some time interval
> after a partial dump.
> Unfortunately two continuous dumps would be likely to suppress
> it forever. Maybe sleep user space dump requests for the first
> block while any resize (esp. grow) is pending.
> 
> What is passed to userspace as the 'continue from here' marker?
> Even without resize there are likely to be issues if something
> nearer the head of a hash chain being processed is deleted.

I assumed that the proposal would include a timer in the Netlink
cb which fires after 2s or so and cancels the dump. 2s is still
a long time and a lot of inserts can happen in the meantime.

This may become more viable once insert()s can fail above a
certain watermark as Herbert suggests.

^ permalink raw reply

* RE: [PATCH 3/3] netlink: Lock out table resizes while dumping Netlink sockets
From: David Laight @ 2015-01-20 15:00 UTC (permalink / raw)
  To: 'Patrick McHardy', Thomas Graf
  Cc: davem@davemloft.net, herbert@gondor.apana.org.au,
	paulmck@linux.vnet.ibm.com, ying.xue@windriver.com,
	netdev@vger.kernel.org, netfilter-devel@vger.kernel.org
In-Reply-To: <20150120143154.GR14883@acer.localdomain>

From: Patrick McHardy
> On 20.01, Thomas Graf wrote:
> > Lock out table resizes while dumping Netlink sockets to user space.
> > This keeps disruptions to a minimum for readers which don't handle
> > the NLM_F_DUMP_INTR flag.
> 
> This doesn't lock them out for the duration of the entire dump of
> course, so the benefit seems rather small. Still with this patch,
> they will need to handle NLM_F_DUMP_INTR or will get unpredictable
> behaviour, in which case I'd think it makes more sense to not even
> try this, all it does is hide parts of the brokenness.
> 
> An alternative would be to set a flag in ht when a dump begins that
> indicates to skip resizing operations and on the end of the dump
> perform any resizing operations that might be necessary. Herbert
> disagrees though and he might be right.

Can you rely on being told when the dump completes?
If the program is killed in the middle then it can't tell you.

I suspect you'd have to suppress resize for some time interval
after a partial dump.
Unfortunately two continuous dumps would be likely to suppress
it forever. Maybe sleep user space dump requests for the first
block while any resize (esp. grow) is pending.

What is passed to userspace as the 'continue from here' marker?
Even without resize there are likely to be issues if something
nearer the head of a hash chain being processed is deleted.

	David


^ permalink raw reply

* Re: [PATCH 3/3] netlink: Lock out table resizes while dumping Netlink sockets
From: Thomas Graf @ 2015-01-20 14:55 UTC (permalink / raw)
  To: Patrick McHardy
  Cc: davem, herbert, paulmck, ying.xue, netdev, netfilter-devel
In-Reply-To: <20150120143154.GR14883@acer.localdomain>

On 01/20/15 at 02:31pm, Patrick McHardy wrote:
> On 20.01, Thomas Graf wrote:
> > Lock out table resizes while dumping Netlink sockets to user space.
> > This keeps disruptions to a minimum for readers which don't handle
> > the NLM_F_DUMP_INTR flag.
> 
> This doesn't lock them out for the duration of the entire dump of
> course, so the benefit seems rather small. Still with this patch,
> they will need to handle NLM_F_DUMP_INTR or will get unpredictable
> behaviour, in which case I'd think it makes more sense to not even
> try this, all it does is hide parts of the brokenness.

If it would lock out the resize for the entire dump I would not have
done patches 1 and 2 ;-)

I does provide better behaviour if the whole dump fits into a single
buffer or if it fits into 2 buffers and we are already dumping into
the 2nd buffer when the resize occurs. Otherwise we will see resizes
and thus tons of duplicates even in those scenarios even if no insert
or removal occurs in parallel.

In the case of Netlink diag that should be typical case. Most systems
will not have 1000s of Netlink sockets in parallel.

> An alternative would be to set a flag in ht when a dump begins that
> indicates to skip resizing operations and on the end of the dump
> perform any resizing operations that might be necessary. Herbert
> disagrees though and he might be right.

I don't like the flag as it prevents resizes (and possibly rehashes
further down the road) for a long period of time. The hashtable
becomes attackable.

^ permalink raw reply

* [PATCHv2 net-next] xen-netback: always fully coalesce guest Rx packets
From: David Vrabel @ 2015-01-20 14:49 UTC (permalink / raw)
  To: netdev; +Cc: David Vrabel, xen-devel, Ian Campbell, Wei Liu

Always fully coalesce guest Rx packets into the minimum number of ring
slots.  Reducing the number of slots per packet has significant
performance benefits when receiving off-host traffic.

Results from XenServer's performance benchmarks:

                         Baseline    Full coalesce
Interhost VM receive      7.2 Gb/s   11 Gb/s
Interhost aggregate      24 Gb/s     24 Gb/s
Intrahost single stream  14 Gb/s     14 Gb/s
Intrahost aggregate      34 Gb/s     34 Gb/s

However, this can increase the number of grant ops per packet which
decreases performance of backend (dom0) to VM traffic (by ~10%)
/unless/ grant copy has been optimized for adjacent ops with the same
source or destination (see "grant-table: defer releasing pages
acquired in a grant copy"[1] expected in Xen 4.6).

[1] http://lists.xen.org/archives/html/xen-devel/2015-01/msg01118.html

Signed-off-by: David Vrabel <david.vrabel@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
Changes in v2:
- Updated commit message with better results.

 drivers/net/xen-netback/common.h  |    1 -
 drivers/net/xen-netback/netback.c |  106 ++-----------------------------------
 2 files changed, 3 insertions(+), 104 deletions(-)

diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
index 5f1fda4..589fa25 100644
--- a/drivers/net/xen-netback/common.h
+++ b/drivers/net/xen-netback/common.h
@@ -251,7 +251,6 @@ struct xenvif {
 struct xenvif_rx_cb {
 	unsigned long expires;
 	int meta_slots_used;
-	bool full_coalesce;
 };
 
 #define XENVIF_RX_CB(skb) ((struct xenvif_rx_cb *)(skb)->cb)
diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index 908e65e..568238d 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -233,51 +233,6 @@ static void xenvif_rx_queue_drop_expired(struct xenvif_queue *queue)
 	}
 }
 
-/*
- * Returns true if we should start a new receive buffer instead of
- * adding 'size' bytes to a buffer which currently contains 'offset'
- * bytes.
- */
-static bool start_new_rx_buffer(int offset, unsigned long size, int head,
-				bool full_coalesce)
-{
-	/* simple case: we have completely filled the current buffer. */
-	if (offset == MAX_BUFFER_OFFSET)
-		return true;
-
-	/*
-	 * complex case: start a fresh buffer if the current frag
-	 * would overflow the current buffer but only if:
-	 *     (i)   this frag would fit completely in the next buffer
-	 * and (ii)  there is already some data in the current buffer
-	 * and (iii) this is not the head buffer.
-	 * and (iv)  there is no need to fully utilize the buffers
-	 *
-	 * Where:
-	 * - (i) stops us splitting a frag into two copies
-	 *   unless the frag is too large for a single buffer.
-	 * - (ii) stops us from leaving a buffer pointlessly empty.
-	 * - (iii) stops us leaving the first buffer
-	 *   empty. Strictly speaking this is already covered
-	 *   by (ii) but is explicitly checked because
-	 *   netfront relies on the first buffer being
-	 *   non-empty and can crash otherwise.
-	 * - (iv) is needed for skbs which can use up more than MAX_SKB_FRAGS
-	 *   slot
-	 *
-	 * This means we will effectively linearise small
-	 * frags but do not needlessly split large buffers
-	 * into multiple copies tend to give large frags their
-	 * own buffers as before.
-	 */
-	BUG_ON(size > MAX_BUFFER_OFFSET);
-	if ((offset + size > MAX_BUFFER_OFFSET) && offset && !head &&
-	    !full_coalesce)
-		return true;
-
-	return false;
-}
-
 struct netrx_pending_operations {
 	unsigned copy_prod, copy_cons;
 	unsigned meta_prod, meta_cons;
@@ -336,24 +291,13 @@ static void xenvif_gop_frag_copy(struct xenvif_queue *queue, struct sk_buff *skb
 		BUG_ON(offset >= PAGE_SIZE);
 		BUG_ON(npo->copy_off > MAX_BUFFER_OFFSET);
 
-		bytes = PAGE_SIZE - offset;
+		if (npo->copy_off == MAX_BUFFER_OFFSET)
+			meta = get_next_rx_buffer(queue, npo);
 
+		bytes = PAGE_SIZE - offset;
 		if (bytes > size)
 			bytes = size;
 
-		if (start_new_rx_buffer(npo->copy_off,
-					bytes,
-					*head,
-					XENVIF_RX_CB(skb)->full_coalesce)) {
-			/*
-			 * Netfront requires there to be some data in the head
-			 * buffer.
-			 */
-			BUG_ON(*head);
-
-			meta = get_next_rx_buffer(queue, npo);
-		}
-
 		if (npo->copy_off + bytes > MAX_BUFFER_OFFSET)
 			bytes = MAX_BUFFER_OFFSET - npo->copy_off;
 
@@ -652,60 +596,16 @@ static void xenvif_rx_action(struct xenvif_queue *queue)
 
 	while (xenvif_rx_ring_slots_available(queue, XEN_NETBK_RX_SLOTS_MAX)
 	       && (skb = xenvif_rx_dequeue(queue)) != NULL) {
-		RING_IDX max_slots_needed;
 		RING_IDX old_req_cons;
 		RING_IDX ring_slots_used;
 		int i;
 
 		queue->last_rx_time = jiffies;
 
-		/* We need a cheap worse case estimate for the number of
-		 * slots we'll use.
-		 */
-
-		max_slots_needed = DIV_ROUND_UP(offset_in_page(skb->data) +
-						skb_headlen(skb),
-						PAGE_SIZE);
-		for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
-			unsigned int size;
-			unsigned int offset;
-
-			size = skb_frag_size(&skb_shinfo(skb)->frags[i]);
-			offset = skb_shinfo(skb)->frags[i].page_offset;
-
-			/* For a worse-case estimate we need to factor in
-			 * the fragment page offset as this will affect the
-			 * number of times xenvif_gop_frag_copy() will
-			 * call start_new_rx_buffer().
-			 */
-			max_slots_needed += DIV_ROUND_UP(offset + size,
-							 PAGE_SIZE);
-		}
-
-		/* To avoid the estimate becoming too pessimal for some
-		 * frontends that limit posted rx requests, cap the estimate
-		 * at MAX_SKB_FRAGS. In this case netback will fully coalesce
-		 * the skb into the provided slots.
-		 */
-		if (max_slots_needed > MAX_SKB_FRAGS) {
-			max_slots_needed = MAX_SKB_FRAGS;
-			XENVIF_RX_CB(skb)->full_coalesce = true;
-		} else {
-			XENVIF_RX_CB(skb)->full_coalesce = false;
-		}
-
-		/* We may need one more slot for GSO metadata */
-		if (skb_is_gso(skb) &&
-		   (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV4 ||
-		    skb_shinfo(skb)->gso_type & SKB_GSO_TCPV6))
-			max_slots_needed++;
-
 		old_req_cons = queue->rx.req_cons;
 		XENVIF_RX_CB(skb)->meta_slots_used = xenvif_gop_skb(skb, &npo, queue);
 		ring_slots_used = queue->rx.req_cons - old_req_cons;
 
-		BUG_ON(ring_slots_used > max_slots_needed);
-
 		__skb_queue_tail(&rxq, skb);
 	}
 
-- 
1.7.10.4

^ permalink raw reply related

* Re: [PATCH net 2/2] bnx2x: fix napi poll return value for repoll
From: Eric Dumazet @ 2015-01-20 14:54 UTC (permalink / raw)
  To: Govindarajulu Varadarajan
  Cc: Willem de Bruijn, davem, netdev, ariel.elior, ssujith, benve
In-Reply-To: <1421765480.4832.2.camel@edumazet-glaptop2.roam.corp.google.com>

On Tue, 2015-01-20 at 06:51 -0800, Eric Dumazet wrote:

> >  
> >  		for_each_cos_in_tx_queue(fp, cos)
> >  			if (bnx2x_tx_queue_has_work(fp->txdata_ptr[cos]))
> > @@ -3187,7 +3187,7 @@ static int bnx2x_poll(struct napi_struct *napi, int budget)
> >  			/* must not complete if we consumed full budget */
> >  			if (work_done >= budget) {
> >  				bnx2x_fp_unlock_napi(fp);
> > -				break;
> > +				return budget;
> 
> This one looks fine.

But its not necessary, as here budget == work_done.

(work_done > budget) would be a bug from bnx2x_rx_int()

^ permalink raw reply

* Re: [PATCH net 2/2] bnx2x: fix napi poll return value for repoll
From: Eric Dumazet @ 2015-01-20 14:51 UTC (permalink / raw)
  To: Govindarajulu Varadarajan, Willem de Bruijn
  Cc: davem, netdev, ariel.elior, ssujith, benve
In-Reply-To: <1421759776-376-3-git-send-email-_govind@gmx.com>

On Tue, 2015-01-20 at 18:46 +0530, Govindarajulu Varadarajan wrote:
> With the commit d75b1ade567ffab ("net: less interrupt masking in NAPI") napi repoll
> is done only when work_done == budget. When in busy_poll is we return 0 in
> napi_poll. We should return budget. Also do not return workdone > budget.
> 

I am not sure.

> Signed-off-by: Govindarajulu Varadarajan <_govind@gmx.com>
> ---
>  drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> index 1d1147c..ebcbe92 100644
> --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> @@ -3175,7 +3175,7 @@ static int bnx2x_poll(struct napi_struct *napi, int budget)
>  		}
>  #endif
>  		if (!bnx2x_fp_lock_napi(fp))
> -			return work_done;
> +			return budget;

For this one I am not sure.

Busy poll is not supposed to drain the whole queue.

>  
>  		for_each_cos_in_tx_queue(fp, cos)
>  			if (bnx2x_tx_queue_has_work(fp->txdata_ptr[cos]))
> @@ -3187,7 +3187,7 @@ static int bnx2x_poll(struct napi_struct *napi, int budget)
>  			/* must not complete if we consumed full budget */
>  			if (work_done >= budget) {
>  				bnx2x_fp_unlock_napi(fp);
> -				break;
> +				return budget;

This one looks fine.

>  			}
>  		}
>  

^ permalink raw reply

* Re: [PATCH 3/3] netlink: Lock out table resizes while dumping Netlink sockets
From: Patrick McHardy @ 2015-01-20 14:31 UTC (permalink / raw)
  To: Thomas Graf; +Cc: davem, herbert, paulmck, ying.xue, netdev, netfilter-devel
In-Reply-To: <e164edeacaeffee43be64d4ed6a5b5e7e8923bd1.1421759056.git.tgraf@suug.ch>

On 20.01, Thomas Graf wrote:
> Lock out table resizes while dumping Netlink sockets to user space.
> This keeps disruptions to a minimum for readers which don't handle
> the NLM_F_DUMP_INTR flag.

This doesn't lock them out for the duration of the entire dump of
course, so the benefit seems rather small. Still with this patch,
they will need to handle NLM_F_DUMP_INTR or will get unpredictable
behaviour, in which case I'd think it makes more sense to not even
try this, all it does is hide parts of the brokenness.

An alternative would be to set a flag in ht when a dump begins that
indicates to skip resizing operations and on the end of the dump
perform any resizing operations that might be necessary. Herbert
disagrees though and he might be right.

> Signed-off-by: Thomas Graf <tgraf@suug.ch>
> ---
>  net/netlink/diag.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/net/netlink/diag.c b/net/netlink/diag.c
> index 50aa385..be4ea6e 100644
> --- a/net/netlink/diag.c
> +++ b/net/netlink/diag.c
> @@ -114,6 +114,8 @@ static int __netlink_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
>  	req = nlmsg_data(cb->nlh);
>  	cb->seq = atomic_read(&nl_table_seq);
>  
> +	mutex_lock(&ht->mutex);
> +
>  	for (i = 0; i < htbl->size; i++) {
>  		struct rhash_head *pos;
>  
> @@ -161,6 +163,7 @@ static int __netlink_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
>  		num++;
>  	}
>  done:
> +	mutex_unlock(&ht->mutex);
>  	cb->args[0] = num;
>  	cb->args[1] = protocol;
>  
> -- 
> 1.9.3
> 

^ permalink raw reply

* Re: [net PATCH 1/1] drivers: net: cpsw: discard dual emac default vlan configuration
From: Sergei Shtylyov @ 2015-01-20 14:25 UTC (permalink / raw)
  To: Mugunthan V N, netdev; +Cc: davem, stable
In-Reply-To: <1421756740-32180-1-git-send-email-mugunthanvnm@ti.com>

Hello.

On 1/20/2015 3:25 PM, Mugunthan V N wrote:

> In Dual EMAC, the default vlans are used to segregate rx packets between

    VLANs. And RX.

> the ports, so adding the same default vlan to the switch will affect the

    VLAN.

> normal packet transfers. So returning error on addition of dual emac

    EMAC.

> default vlans.

    VLANs.

> Even if emac 0 default port vlan is added to emac 1, it will lead to

    EMAC. And VLAN

> break

    Breaking.

> dual EMAC port seperations.

    Separations.

> Fixes: d9ba8f9 (driver: net: ethernet: cpsw: dual emac interface implementation)

    12 hex digits, please.

> Cc: <stable@vger.kernel.org> # v3.9+
> Reported-by: Felipe Balbi <balbi@ti.com>
> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
> ---
>   drivers/net/ethernet/ti/cpsw.c | 22 ++++++++++++++++++++++
>   1 file changed, 22 insertions(+)

> diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
> index e068d48..7c32815 100644
> --- a/drivers/net/ethernet/ti/cpsw.c
> +++ b/drivers/net/ethernet/ti/cpsw.c
> @@ -1683,6 +1683,19 @@ static int cpsw_ndo_vlan_rx_add_vid(struct net_device *ndev,
>   	if (vid == priv->data.default_vlan)
>   		return 0;
>
> +	if (priv->data.dual_emac) {
> +		/* In dual EMAC, reserved VLAN id should not be used of

    s/of/for/, perhaps?

> +		 * creating vlan interfaces as this can break the dual

    s/vlan/VLAN/. Be consistent, please.

> +		 * EMAC port seperation

    Separation.

[...]

WBR, Sergei

^ permalink raw reply

* [PATCH 2/2] ss: Unify inet sockets output
From: Vadim Kochan @ 2015-01-20 14:14 UTC (permalink / raw)
  To: netdev; +Cc: Vadim Kochan
In-Reply-To: <1421763264-8954-1-git-send-email-vadim4j@gmail.com>

From: Vadim Kochan <vadim4j@gmail.com>

Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
---
 misc/ss.c | 677 ++++++++++++++++++++++++++++++++------------------------------
 1 file changed, 355 insertions(+), 322 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index 3764f3a..ef1fc9d 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -689,23 +689,59 @@ static const char *sstate_namel[] = {
 	[SS_CLOSING] = "closing",
 };
 
+struct dctcpstat
+{
+	unsigned int	ce_state;
+	unsigned int	alpha;
+	unsigned int	ab_ecn;
+	unsigned int	ab_tot;
+	bool		enabled;
+};
+
 struct tcpstat
 {
-	inet_prefix	local;
-	inet_prefix	remote;
-	int		lport;
-	int		rport;
-	int		state;
-	int		rq, wq;
-	int		timer;
-	int		timeout;
-	int		retrs;
-	unsigned	ino;
-	int		probes;
-	unsigned	uid;
-	int		refcnt;
-	unsigned long long sk;
-	int		rto, ato, qack, cwnd, ssthresh;
+	inet_prefix	    local;
+	inet_prefix	    remote;
+	int		    lport;
+	int		    rport;
+	int		    state;
+	int		    rq, wq;
+	unsigned	    ino;
+	unsigned	    uid;
+	int		    refcnt;
+	unsigned int	    iface;
+	unsigned long long  sk;
+	int		    timer;
+	int		    timeout;
+	int		    probes;
+	char		    *cong_alg;
+	double		    rto, ato, rtt, rttvar;
+	int		    qack, cwnd, ssthresh, backoff;
+	double		    send_bps;
+	int		    snd_wscale;
+	int		    rcv_wscale;
+	int		    mss;
+	unsigned int	    lastsnd;
+	unsigned int	    lastrcv;
+	unsigned int	    lastack;
+	double		    pacing_rate;
+	double		    pacing_rate_max;
+	unsigned int	    unacked;
+	unsigned int	    retrans;
+	unsigned int	    retrans_total;
+	unsigned int	    lost;
+	unsigned int	    sacked;
+	unsigned int	    fackets;
+	unsigned int	    reordering;
+	double		    rcv_rtt;
+	int		    rcv_space;
+	bool		    has_ts_opt;
+	bool		    has_sack_opt;
+	bool		    has_ecn_opt;
+	bool		    has_ecnseen_opt;
+	bool		    has_fastopen_opt;
+	bool		    has_wscale_opt;
+	struct dctcpstat    *dctcp;
 };
 
 static const char *tmr_name[] = {
@@ -744,12 +780,6 @@ static const char *print_ms_timer(int timeout)
 	return buf;
 }
 
-static const char *print_hz_timer(int timeout)
-{
-	int hz = get_user_hz();
-	return print_ms_timer(((timeout*1000) + hz-1)/hz);
-}
-
 struct scache
 {
 	struct scache *next;
@@ -1439,55 +1469,220 @@ out:
 	return res;
 }
 
-static int tcp_show_line(char *line, const struct filter *f, int family)
+static char *proto_name(int protocol)
+{
+	switch (protocol) {
+	case IPPROTO_UDP:
+		return "udp";
+	case IPPROTO_TCP:
+		return "tcp";
+	case IPPROTO_DCCP:
+		return "dccp";
+	}
+
+	return "???";
+}
+
+static void inet_stats_print(struct tcpstat *s, int protocol)
+{
+	char *buf = NULL;
+
+	if (netid_width)
+		printf("%-*s ", netid_width, proto_name(protocol));
+	if (state_width)
+		printf("%-*s ", state_width, sstate_name[s->state]);
+
+	printf("%-6d %-6d ", s->rq, s->wq);
+
+	formatted_print(&s->local, s->lport, s->iface);
+	formatted_print(&s->remote, s->rport, 0);
+
+	if (show_options) {
+		if (s->timer) {
+			if (s->timer > 4)
+				s->timer = 5;
+			printf(" timer:(%s,%s,%d)",
+			       tmr_name[s->timer],
+			       print_ms_timer(s->timeout),
+			       s->retrans);
+		}
+	}
+
+	if (show_proc_ctx || show_sock_ctx) {
+		if (find_entry(s->ino, &buf,
+				(show_proc_ctx & show_sock_ctx) ?
+				PROC_SOCK_CTX : PROC_CTX) > 0) {
+			printf(" users:(%s)", buf);
+			free(buf);
+		}
+	} else if (show_users) {
+		if (find_entry(s->ino, &buf, USERS) > 0) {
+			printf(" users:(%s)", buf);
+			free(buf);
+		}
+	}
+}
+
+static int proc_parse_inet_addr(char *loc, char *rem, int family, struct tcpstat *s)
+{
+	s->local.family = s->remote.family = family;
+	if (family == AF_INET) {
+		sscanf(loc, "%x:%x", s->local.data, (unsigned*)&s->lport);
+		sscanf(rem, "%x:%x", s->remote.data, (unsigned*)&s->rport);
+		s->local.bytelen = s->remote.bytelen = 4;
+		return 0;
+	} else {
+		sscanf(loc, "%08x%08x%08x%08x:%x",
+		       s->local.data,
+		       s->local.data + 1,
+		       s->local.data + 2,
+		       s->local.data + 3,
+		       &s->lport);
+		sscanf(rem, "%08x%08x%08x%08x:%x",
+		       s->remote.data,
+		       s->remote.data + 1,
+		       s->remote.data + 2,
+		       s->remote.data + 3,
+		       &s->rport);
+		s->local.bytelen = s->remote.bytelen = 16;
+		return 0;
+	}
+	return -1;
+}
+
+static int proc_inet_split_line(char *line, char **loc, char **rem, char **data)
 {
-	struct tcpstat s;
-	char *loc, *rem, *data;
-	char opt[256];
-	int n;
 	char *p;
 
 	if ((p = strchr(line, ':')) == NULL)
 		return -1;
-	loc = p+2;
 
-	if ((p = strchr(loc, ':')) == NULL)
+	*loc = p+2;
+	if ((p = strchr(*loc, ':')) == NULL)
 		return -1;
-	p[5] = 0;
-	rem = p+6;
 
-	if ((p = strchr(rem, ':')) == NULL)
+	p[5] = 0;
+	*rem = p+6;
+	if ((p = strchr(*rem, ':')) == NULL)
 		return -1;
+
 	p[5] = 0;
-	data = p+6;
+	*data = p+6;
+	return 0;
+}
 
-	do {
-		int state = (data[1] >= 'A') ? (data[1] - 'A' + 10) : (data[1] - '0');
+static char *sprint_bw(char *buf, double bw)
+{
+	if (bw > 1000000.)
+		sprintf(buf,"%.1fM", bw / 1000000.);
+	else if (bw > 1000.)
+		sprintf(buf,"%.1fK", bw / 1000.);
+	else
+		sprintf(buf, "%g", bw);
 
-		if (!(f->states & (1<<state)))
-			return 0;
-	} while (0);
+	return buf;
+}
 
-	s.local.family = s.remote.family = family;
-	if (family == AF_INET) {
-		sscanf(loc, "%x:%x", s.local.data, (unsigned*)&s.lport);
-		sscanf(rem, "%x:%x", s.remote.data, (unsigned*)&s.rport);
-		s.local.bytelen = s.remote.bytelen = 4;
-	} else {
-		sscanf(loc, "%08x%08x%08x%08x:%x",
-		       s.local.data,
-		       s.local.data+1,
-		       s.local.data+2,
-		       s.local.data+3,
-		       &s.lport);
-		sscanf(rem, "%08x%08x%08x%08x:%x",
-		       s.remote.data,
-		       s.remote.data+1,
-		       s.remote.data+2,
-		       s.remote.data+3,
-		       &s.rport);
-		s.local.bytelen = s.remote.bytelen = 16;
-	}
+static void tcp_stats_print(struct tcpstat *s)
+{
+	char b1[64];
+
+	if (s->has_ts_opt)
+		printf(" ts");
+	if (s->has_sack_opt)
+		printf(" sack");
+	if (s->has_ecn_opt)
+		printf(" ecn");
+	if (s->has_ecnseen_opt)
+		printf(" ecnseen");
+	if (s->has_fastopen_opt)
+		printf(" fastopen");
+	if (s->cong_alg)
+		printf(" %s", s->cong_alg);
+	if (s->has_wscale_opt)
+		printf(" wscale:%d,%d", s->snd_wscale, s->rcv_wscale);
+	if (s->rto)
+		printf(" rto:%g", s->rto);
+	if (s->backoff)
+		printf(" backoff:%u", s->backoff);
+	if (s->rtt)
+		printf(" rtt:%g/%g", s->rtt, s->rttvar);
+	if (s->ato)
+		printf(" ato:%g", s->ato);
+
+	if (s->qack)
+		printf(" qack:%d", s->qack);
+	if (s->qack & 1)
+		printf(" bidir");
+
+	if (s->mss)
+		printf(" mss:%d", s->mss);
+	if (s->cwnd && s->cwnd != 2)
+		printf(" cwnd:%d", s->cwnd);
+	if (s->ssthresh)
+		printf(" ssthresh:%d", s->ssthresh);
+
+	if (s->dctcp && s->dctcp->enabled) {
+		struct dctcpstat *dctcp = s->dctcp;
+
+		printf(" ce_state %u alpha %u ab_ecn %u ab_tot %u",
+				dctcp->ce_state, dctcp->alpha, dctcp->ab_ecn,
+				dctcp->ab_tot);
+	} else if (s->dctcp) {
+		printf(" fallback_mode");
+	}
+
+	if (s->send_bps)
+		printf(" send %sbps", sprint_bw(b1, s->send_bps));
+	if (s->lastsnd)
+		printf(" lastsnd:%u", s->lastsnd);
+	if (s->lastrcv)
+		printf(" lastrcv:%u", s->lastrcv);
+	if (s->lastack)
+		printf(" lastack:%u", s->lastack);
+
+	if (s->pacing_rate) {
+		printf(" pacing_rate %sbps", sprint_bw(b1, s->pacing_rate));
+		if (s->pacing_rate_max)
+				printf("/%sbps", sprint_bw(b1,
+							s->pacing_rate_max));
+	}
+
+	if (s->unacked)
+		printf(" unacked:%u", s->unacked);
+	if (s->retrans || s->retrans_total)
+		printf(" retrans:%u/%u", s->retrans, s->retrans_total);
+	if (s->lost)
+		printf(" lost:%u", s->lost);
+	if (s->sacked && s->state != SS_LISTEN)
+		printf(" sacked:%u", s->sacked);
+	if (s->fackets)
+		printf(" fackets:%u", s->fackets);
+	if (s->reordering != 3)
+		printf(" reordering:%d", s->reordering);
+	if (s->rcv_rtt)
+		printf(" rcv_rtt:%g", s->rcv_rtt);
+	if (s->rcv_space)
+		printf(" rcv_space:%d", s->rcv_space);
+}
+
+static int tcp_show_line(char *line, const struct filter *f, int family)
+{
+	int rto = 0, ato = 0;
+	struct tcpstat s = {};
+	char *loc, *rem, *data;
+	char opt[256];
+	int n;
+	int hz = get_user_hz();
+
+	if (proc_inet_split_line(line, &loc, &rem, &data))
+		return -1;
+
+	int state = (data[1] >= 'A') ? (data[1] - 'A' + 10) : (data[1] - '0');
+	if (!(f->states & (1 << state)))
+		return 0;
+
+	proc_parse_inet_addr(loc, rem, family, &s);
 
 	if (f->f && run_ssfilter(f->f, &s) == 0)
 		return 0;
@@ -1495,69 +1690,29 @@ static int tcp_show_line(char *line, const struct filter *f, int family)
 	opt[0] = 0;
 	n = sscanf(data, "%x %x:%x %x:%x %x %d %d %u %d %llx %d %d %d %d %d %[^\n]\n",
 		   &s.state, &s.wq, &s.rq,
-		   &s.timer, &s.timeout, &s.retrs, &s.uid, &s.probes, &s.ino,
-		   &s.refcnt, &s.sk, &s.rto, &s.ato, &s.qack,
+		   &s.timer, &s.timeout, &s.retrans, &s.uid, &s.probes, &s.ino,
+		   &s.refcnt, &s.sk, &rto, &ato, &s.qack,
 		   &s.cwnd, &s.ssthresh, opt);
 
 	if (n < 17)
 		opt[0] = 0;
 
 	if (n < 12) {
-		s.rto = 0;
+		rto = 0;
 		s.cwnd = 2;
 		s.ssthresh = -1;
-		s.ato = s.qack = 0;
+		ato = s.qack = 0;
 	}
 
-	if (netid_width)
-		printf("%-*s ", netid_width, "tcp");
-	if (state_width)
-		printf("%-*s ", state_width, sstate_name[s.state]);
+	s.retrans   = s.timer != 1 ? s.probes : s.retrans;
+	s.timeout   = (s.timeout * 1000 + hz - 1) / hz;
+	s.ato	    = (double)ato / hz;
+	s.qack	   /= 2;
+	s.rto	    = (double)rto;
+	s.ssthresh  = s.ssthresh == -1 ? 0 : s.ssthresh;
+	s.rto	    = s.rto != 3 * hz  ? s.rto / hz : 0;
 
-	printf("%-6d %-6d ", s.rq, s.wq);
-
-	formatted_print(&s.local, s.lport, 0);
-	formatted_print(&s.remote, s.rport, 0);
-
-	if (show_options) {
-		if (s.timer) {
-			if (s.timer > 4)
-				s.timer = 5;
-			printf(" timer:(%s,%s,%d)",
-			       tmr_name[s.timer],
-			       print_hz_timer(s.timeout),
-			       s.timer != 1 ? s.probes : s.retrs);
-		}
-	}
-	if (show_tcpinfo) {
-		int hz = get_user_hz();
-		if (s.rto && s.rto != 3*hz)
-			printf(" rto:%g", (double)s.rto/hz);
-		if (s.ato)
-			printf(" ato:%g", (double)s.ato/hz);
-		if (s.cwnd != 2)
-			printf(" cwnd:%d", s.cwnd);
-		if (s.ssthresh != -1)
-			printf(" ssthresh:%d", s.ssthresh);
-		if (s.qack/2)
-			printf(" qack:%d", s.qack/2);
-		if (s.qack&1)
-			printf(" bidir");
-	}
-	char *buf = NULL;
-	if (show_proc_ctx || show_sock_ctx) {
-		if (find_entry(s.ino, &buf,
-				(show_proc_ctx & show_sock_ctx) ?
-				PROC_SOCK_CTX : PROC_CTX) > 0) {
-			printf(" users:(%s)", buf);
-			free(buf);
-		}
-	} else if (show_users) {
-		if (find_entry(s.ino, &buf, USERS) > 0) {
-			printf(" users:(%s)", buf);
-			free(buf);
-		}
-	}
+	inet_stats_print(&s, IPPROTO_TCP);
 
 	if (show_details) {
 		if (s.uid)
@@ -1567,8 +1722,11 @@ static int tcp_show_line(char *line, const struct filter *f, int family)
 		if (opt[0])
 			printf(" opt:\"%s\"", opt);
 	}
-	printf("\n");
 
+	if (show_tcpinfo)
+		tcp_stats_print(&s);
+
+	printf("\n");
 	return 0;
 }
 
@@ -1598,18 +1756,6 @@ outerr:
 	return ferror(fp) ? -1 : 0;
 }
 
-static char *sprint_bw(char *buf, double bw)
-{
-	if (bw > 1000000.)
-		sprintf(buf,"%.1fM", bw / 1000000.);
-	else if (bw > 1000.)
-		sprintf(buf,"%.1fK", bw / 1000.);
-	else
-		sprintf(buf, "%g", bw);
-
-	return buf;
-}
-
 static void print_skmeminfo(struct rtattr *tb[], int attrtype)
 {
 	const __u32 *skmeminfo;
@@ -1649,11 +1795,13 @@ static void print_skmeminfo(struct rtattr *tb[], int attrtype)
 	printf(")");
 }
 
+#define TCPI_HAS_OPT(info, opt) !!(info->tcpi_options & (opt))
+
 static void tcp_show_info(const struct nlmsghdr *nlh, struct inet_diag_msg *r,
 		struct rtattr *tb[])
 {
-	char b1[64];
 	double rtt = 0;
+	struct tcpstat s = {};
 
 	print_skmeminfo(tb, INET_DIAG_SKMEMINFO);
 
@@ -1670,39 +1818,49 @@ static void tcp_show_info(const struct nlmsghdr *nlh, struct inet_diag_msg *r,
 			info = RTA_DATA(tb[INET_DIAG_INFO]);
 
 		if (show_options) {
-			if (info->tcpi_options & TCPI_OPT_TIMESTAMPS)
-				printf(" ts");
-			if (info->tcpi_options & TCPI_OPT_SACK)
-				printf(" sack");
-			if (info->tcpi_options & TCPI_OPT_ECN)
-				printf(" ecn");
-			if (info->tcpi_options & TCPI_OPT_ECN_SEEN)
-				printf(" ecnseen");
-			if (info->tcpi_options & TCPI_OPT_SYN_DATA)
-				printf(" fastopen");
-		}
-
-		if (tb[INET_DIAG_CONG])
-			printf(" %s", rta_getattr_str(tb[INET_DIAG_CONG]));
-
-		if (info->tcpi_options & TCPI_OPT_WSCALE)
-			printf(" wscale:%d,%d", info->tcpi_snd_wscale,
-			       info->tcpi_rcv_wscale);
+			s.has_ts_opt	   = TCPI_HAS_OPT(info, TCPI_OPT_TIMESTAMPS);
+			s.has_sack_opt	   = TCPI_HAS_OPT(info, TCPI_OPT_SACK);
+			s.has_ecn_opt	   = TCPI_HAS_OPT(info, TCPI_OPT_ECN);
+			s.has_ecnseen_opt  = TCPI_HAS_OPT(info, TCPI_OPT_ECN_SEEN);
+			s.has_fastopen_opt = TCPI_HAS_OPT(info, TCPI_OPT_SYN_DATA);
+		}
+
+		if (tb[INET_DIAG_CONG]) {
+			const char *cong_attr = rta_getattr_str(tb[INET_DIAG_CONG]);
+			s.cong_alg = malloc(strlen(cong_attr + 1));
+			strcpy(s.cong_alg, cong_attr);
+		}
+
+		if (TCPI_HAS_OPT(info, TCPI_OPT_WSCALE)) {
+			s.has_wscale_opt  = true;
+			s.snd_wscale	  = info->tcpi_snd_wscale;
+			s.rcv_wscale	  = info->tcpi_rcv_wscale;
+		}
+
 		if (info->tcpi_rto && info->tcpi_rto != 3000000)
-			printf(" rto:%g", (double)info->tcpi_rto/1000);
-		if (info->tcpi_backoff)
-			printf(" backoff:%u", info->tcpi_backoff);
-		if (info->tcpi_rtt)
-			printf(" rtt:%g/%g", (double)info->tcpi_rtt/1000,
-			       (double)info->tcpi_rttvar/1000);
-		if (info->tcpi_ato)
-			printf(" ato:%g", (double)info->tcpi_ato/1000);
-		if (info->tcpi_snd_mss)
-			printf(" mss:%d", info->tcpi_snd_mss);
-		if (info->tcpi_snd_cwnd != 2)
-			printf(" cwnd:%d", info->tcpi_snd_cwnd);
+			s.rto = (double)info->tcpi_rto / 1000;
+
+		s.backoff	 = info->tcpi_backoff;
+		s.rtt		 = (double)info->tcpi_rtt / 1000;
+		s.rttvar	 = (double)info->tcpi_rttvar / 1000;
+		s.ato		 = (double)info->tcpi_rttvar / 1000;
+		s.mss		 = info->tcpi_snd_mss;
+		s.rcv_space	 = info->tcpi_rcv_space;
+		s.rcv_rtt	 = (double)info->tcpi_rcv_rtt / 1000;
+		s.lastsnd	 = info->tcpi_last_data_sent;
+		s.lastrcv	 = info->tcpi_last_data_recv;
+		s.lastack	 = info->tcpi_last_ack_recv;
+		s.unacked	 = info->tcpi_unacked;
+		s.retrans	 = info->tcpi_retrans;
+		s.retrans_total  = info->tcpi_total_retrans;
+		s.lost		 = info->tcpi_lost;
+		s.sacked	 = info->tcpi_sacked;
+		s.reordering	 = info->tcpi_reordering;
+		s.rcv_space	 = info->tcpi_rcv_space;
+		s.cwnd		 = info->tcpi_snd_cwnd;
+
 		if (info->tcpi_snd_ssthresh < 0xFFFF)
-			printf(" ssthresh:%d", info->tcpi_snd_ssthresh);
+			s.ssthresh = info->tcpi_snd_ssthresh;
 
 		rtt = (double) info->tcpi_rtt;
 		if (tb[INET_DIAG_VEGASINFO]) {
@@ -1710,89 +1868,51 @@ static void tcp_show_info(const struct nlmsghdr *nlh, struct inet_diag_msg *r,
 				= RTA_DATA(tb[INET_DIAG_VEGASINFO]);
 
 			if (vinfo->tcpv_enabled &&
-			    vinfo->tcpv_rtt && vinfo->tcpv_rtt != 0x7fffffff)
+					vinfo->tcpv_rtt && vinfo->tcpv_rtt != 0x7fffffff)
 				rtt =  vinfo->tcpv_rtt;
 		}
 
 		if (tb[INET_DIAG_DCTCPINFO]) {
+			struct dctcpstat *dctcp = malloc(sizeof(struct
+						dctcpstat));
+
 			const struct tcp_dctcp_info *dinfo
 				= RTA_DATA(tb[INET_DIAG_DCTCPINFO]);
 
-			if (dinfo->dctcp_enabled) {
-				printf(" ce_state %u alpha %u ab_ecn %u ab_tot %u",
-				       dinfo->dctcp_ce_state, dinfo->dctcp_alpha,
-				       dinfo->dctcp_ab_ecn, dinfo->dctcp_ab_tot);
-			} else {
-				printf(" fallback_mode");
-			}
+			dctcp->enabled	= !!dinfo->dctcp_enabled;
+			dctcp->ce_state = dinfo->dctcp_ce_state;
+			dctcp->alpha	= dinfo->dctcp_alpha;
+			dctcp->ab_ecn	= dinfo->dctcp_ab_ecn;
+			dctcp->ab_tot	= dinfo->dctcp_ab_tot;
+			s.dctcp		= dctcp;
 		}
 
 		if (rtt > 0 && info->tcpi_snd_mss && info->tcpi_snd_cwnd) {
-			printf(" send %sbps",
-			       sprint_bw(b1, (double) info->tcpi_snd_cwnd *
-					 (double) info->tcpi_snd_mss * 8000000.
-					 / rtt));
+			s.send_bps = (double) info->tcpi_snd_cwnd *
+				(double)info->tcpi_snd_mss * 8000000. / rtt;
 		}
 
-		if (info->tcpi_last_data_sent)
-			printf(" lastsnd:%u", info->tcpi_last_data_sent);
-
-		if (info->tcpi_last_data_recv)
-			printf(" lastrcv:%u", info->tcpi_last_data_recv);
-
-		if (info->tcpi_last_ack_recv)
-			printf(" lastack:%u", info->tcpi_last_ack_recv);
-
 		if (info->tcpi_pacing_rate &&
-		    info->tcpi_pacing_rate != ~0ULL) {
-			printf(" pacing_rate %sbps",
-				sprint_bw(b1, info->tcpi_pacing_rate * 8.));
+				info->tcpi_pacing_rate != ~0ULL) {
+			s.pacing_rate = info->tcpi_pacing_rate * 8.;
 
 			if (info->tcpi_max_pacing_rate &&
-			    info->tcpi_max_pacing_rate != ~0ULL)
-				printf("/%sbps",
-					sprint_bw(b1, info->tcpi_max_pacing_rate * 8.));
-		}
-		if (info->tcpi_unacked)
-			printf(" unacked:%u", info->tcpi_unacked);
-		if (info->tcpi_retrans || info->tcpi_total_retrans)
-			printf(" retrans:%u/%u", info->tcpi_retrans,
-			       info->tcpi_total_retrans);
-		if (info->tcpi_lost)
-			printf(" lost:%u", info->tcpi_lost);
-		if (info->tcpi_sacked && r->idiag_state != SS_LISTEN)
-			printf(" sacked:%u", info->tcpi_sacked);
-		if (info->tcpi_fackets)
-			printf(" fackets:%u", info->tcpi_fackets);
-		if (info->tcpi_reordering != 3)
-			printf(" reordering:%d", info->tcpi_reordering);
-		if (info->tcpi_rcv_rtt)
-			printf(" rcv_rtt:%g", (double) info->tcpi_rcv_rtt/1000);
-		if (info->tcpi_rcv_space)
-			printf(" rcv_space:%d", info->tcpi_rcv_space);
-
-	}
-}
-
-static char *proto_name(int protocol)
-{
-	switch (protocol) {
-	case IPPROTO_UDP:
-		return "udp";
-	case IPPROTO_TCP:
-		return "tcp";
-	case IPPROTO_DCCP:
-		return "dccp";
+					info->tcpi_max_pacing_rate != ~0ULL)
+				s.pacing_rate_max = info->tcpi_max_pacing_rate * 8.;
+		}
+		tcp_stats_print(&s);
+		if (s.dctcp)
+			free(s.dctcp);
+		if (s.cong_alg)
+			free(s.cong_alg);
 	}
-
-	return "???";
 }
 
 static int inet_show_sock(struct nlmsghdr *nlh, struct filter *f, int protocol)
 {
 	struct rtattr * tb[INET_DIAG_MAX+1];
 	struct inet_diag_msg *r = NLMSG_DATA(nlh);
-	struct tcpstat s;
+	struct tcpstat s = {};
 
 	parse_rtattr(tb, INET_DIAG_MAX, (struct rtattr*)(r+1),
 		     nlh->nlmsg_len - NLMSG_LENGTH(sizeof(*r)));
@@ -1801,52 +1921,28 @@ static int inet_show_sock(struct nlmsghdr *nlh, struct filter *f, int protocol)
 	s.local.family = s.remote.family = r->idiag_family;
 	s.lport = ntohs(r->id.idiag_sport);
 	s.rport = ntohs(r->id.idiag_dport);
+	s.wq = r->idiag_wqueue;
+	s.rq = r->idiag_rqueue;
+	s.timer = r->idiag_timer;
+	s.timeout = r->idiag_expires;
+	s.retrans = r->idiag_retrans;
+	s.ino = r->idiag_inode;
+	s.uid = r->idiag_uid;
+	s.iface = r->id.idiag_if;
+
 	if (s.local.family == AF_INET) {
 		s.local.bytelen = s.remote.bytelen = 4;
 	} else {
 		s.local.bytelen = s.remote.bytelen = 16;
 	}
+
 	memcpy(s.local.data, r->id.idiag_src, s.local.bytelen);
 	memcpy(s.remote.data, r->id.idiag_dst, s.local.bytelen);
 
 	if (f && f->f && run_ssfilter(f->f, &s) == 0)
 		return 0;
 
-	if (netid_width)
-		printf("%-*s ", netid_width, proto_name(protocol));
-	if (state_width)
-		printf("%-*s ", state_width, sstate_name[s.state]);
-
-	printf("%-6d %-6d ", r->idiag_rqueue, r->idiag_wqueue);
-
-	formatted_print(&s.local, s.lport, r->id.idiag_if);
-	formatted_print(&s.remote, s.rport, 0);
-
-	if (show_options) {
-		if (r->idiag_timer) {
-			if (r->idiag_timer > 4)
-				r->idiag_timer = 5;
-			printf(" timer:(%s,%s,%d)",
-			       tmr_name[r->idiag_timer],
-			       print_ms_timer(r->idiag_expires),
-			       r->idiag_retrans);
-		}
-	}
-	char *buf = NULL;
-
-	if (show_proc_ctx || show_sock_ctx) {
-		if (find_entry(r->idiag_inode, &buf,
-				(show_proc_ctx & show_sock_ctx) ?
-				PROC_SOCK_CTX : PROC_CTX) > 0) {
-			printf(" users:(%s)", buf);
-			free(buf);
-		}
-	} else if (show_users) {
-		if (find_entry(r->idiag_inode, &buf, USERS) > 0) {
-			printf(" users:(%s)", buf);
-			free(buf);
-		}
-	}
+	inet_stats_print(&s, protocol);
 
 	if (show_details) {
 		if (r->idiag_uid)
@@ -1862,13 +1958,13 @@ static int inet_show_sock(struct nlmsghdr *nlh, struct filter *f, int protocol)
 			printf(" %c-%c", mask & 1 ? '-' : '<', mask & 2 ? '-' : '>');
 		}
 	}
+
 	if (show_mem || show_tcpinfo) {
 		printf("\n\t");
 		tcp_show_info(nlh, r, tb);
 	}
 
 	printf("\n");
-
 	return 0;
 }
 
@@ -2189,53 +2285,19 @@ outerr:
 
 static int dgram_show_line(char *line, const struct filter *f, int family)
 {
-	struct tcpstat s;
+	struct tcpstat s = {};
 	char *loc, *rem, *data;
 	char opt[256];
 	int n;
-	char *p;
-
-	if ((p = strchr(line, ':')) == NULL)
-		return -1;
-	loc = p+2;
 
-	if ((p = strchr(loc, ':')) == NULL)
+	if (proc_inet_split_line(line, &loc, &rem, &data))
 		return -1;
-	p[5] = 0;
-	rem = p+6;
-
-	if ((p = strchr(rem, ':')) == NULL)
-		return -1;
-	p[5] = 0;
-	data = p+6;
 
-	do {
-		int state = (data[1] >= 'A') ? (data[1] - 'A' + 10) : (data[1] - '0');
+	int state = (data[1] >= 'A') ? (data[1] - 'A' + 10) : (data[1] - '0');
+	if (!(f->states & (1 << state)))
+		return 0;
 
-		if (!(f->states & (1<<state)))
-			return 0;
-	} while (0);
-
-	s.local.family = s.remote.family = family;
-	if (family == AF_INET) {
-		sscanf(loc, "%x:%x", s.local.data, (unsigned*)&s.lport);
-		sscanf(rem, "%x:%x", s.remote.data, (unsigned*)&s.rport);
-		s.local.bytelen = s.remote.bytelen = 4;
-	} else {
-		sscanf(loc, "%08x%08x%08x%08x:%x",
-		       s.local.data,
-		       s.local.data+1,
-		       s.local.data+2,
-		       s.local.data+3,
-		       &s.lport);
-		sscanf(rem, "%08x%08x%08x%08x:%x",
-		       s.remote.data,
-		       s.remote.data+1,
-		       s.remote.data+2,
-		       s.remote.data+3,
-		       &s.rport);
-		s.local.bytelen = s.remote.bytelen = 16;
-	}
+	proc_parse_inet_addr(loc, rem, family, &s);
 
 	if (f->f && run_ssfilter(f->f, &s) == 0)
 		return 0;
@@ -2249,31 +2311,7 @@ static int dgram_show_line(char *line, const struct filter *f, int family)
 	if (n < 9)
 		opt[0] = 0;
 
-	if (netid_width)
-		printf("%-*s ", netid_width, dg_proto);
-	if (state_width)
-		printf("%-*s ", state_width, sstate_name[s.state]);
-
-	printf("%-6d %-6d ", s.rq, s.wq);
-
-	formatted_print(&s.local, s.lport, 0);
-	formatted_print(&s.remote, s.rport, 0);
-
-	char *buf = NULL;
-
-	if (show_proc_ctx || show_sock_ctx) {
-		if (find_entry(s.ino, &buf,
-				(show_proc_ctx & show_sock_ctx) ?
-				PROC_SOCK_CTX : PROC_CTX) > 0) {
-			printf(" users:(%s)", buf);
-			free(buf);
-		}
-	} else if (show_users) {
-		if (find_entry(s.ino, &buf, USERS) > 0) {
-			printf(" users:(%s)", buf);
-			free(buf);
-		}
-	}
+	inet_stats_print(&s, IPPROTO_UDP);
 
 	if (show_details) {
 		if (s.uid)
@@ -2283,12 +2321,11 @@ static int dgram_show_line(char *line, const struct filter *f, int family)
 		if (opt[0])
 			printf(" opt:\"%s\"", opt);
 	}
-	printf("\n");
 
+	printf("\n");
 	return 0;
 }
 
-
 static int udp_show(struct filter *f)
 {
 	FILE *fp = NULL;
@@ -2357,7 +2394,6 @@ outerr:
 	} while (0);
 }
 
-
 struct unixstat
 {
 	struct unixstat *next;
@@ -2371,12 +2407,9 @@ struct unixstat
 	char *name;
 };
 
-
-
 int unix_state_map[] = { SS_CLOSE, SS_SYN_SENT,
 			 SS_ESTABLISHED, SS_CLOSING };
 
-
 #define MAX_UNIX_REMEMBER (1024*1024/sizeof(struct unixstat))
 
 static void unix_list_free(struct unixstat *list)
-- 
2.1.3

^ permalink raw reply related

* [PATCH 0/2] Refactoring for meminfo & inet sockets output
From: Vadim Kochan @ 2015-01-20 14:14 UTC (permalink / raw)
  To: netdev; +Cc: Vadim Kochan

From: Vadim Kochan <vadim4j@gmail.com>

This is just refactoring to have meminfo and inet stats
output in one place.

Vadim Kochan (2):
  ss: Unify meminfo output
  ss: Unify inet sockets output

 misc/ss.c | 707 +++++++++++++++++++++++++++++++++-----------------------------
 1 file changed, 373 insertions(+), 334 deletions(-)

-- 
2.1.3

^ permalink raw reply

* [PATCH 1/2] ss: Unify meminfo output
From: Vadim Kochan @ 2015-01-20 14:14 UTC (permalink / raw)
  To: netdev; +Cc: Vadim Kochan
In-Reply-To: <1421763264-8954-1-git-send-email-vadim4j@gmail.com>

From: Vadim Kochan <vadim4j@gmail.com>

Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
---
 misc/ss.c | 30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index f434f57..3764f3a 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -1613,8 +1613,24 @@ static char *sprint_bw(char *buf, double bw)
 static void print_skmeminfo(struct rtattr *tb[], int attrtype)
 {
 	const __u32 *skmeminfo;
-	if (!tb[attrtype])
+
+	if (!tb[attrtype]) {
+		if (attrtype == INET_DIAG_SKMEMINFO) {
+			if (!tb[INET_DIAG_MEMINFO])
+				return;
+
+			const struct inet_diag_meminfo *minfo =
+				RTA_DATA(tb[INET_DIAG_MEMINFO]);
+
+			printf(" mem:(r%u,w%u,f%u,t%u)",
+					minfo->idiag_rmem,
+					minfo->idiag_wmem,
+					minfo->idiag_fmem,
+					minfo->idiag_tmem);
+		}
 		return;
+	}
+
 	skmeminfo = RTA_DATA(tb[attrtype]);
 
 	printf(" skmem:(r%u,rb%u,t%u,tb%u,f%u,w%u,o%u",
@@ -1639,17 +1655,7 @@ static void tcp_show_info(const struct nlmsghdr *nlh, struct inet_diag_msg *r,
 	char b1[64];
 	double rtt = 0;
 
-	if (tb[INET_DIAG_SKMEMINFO]) {
-		print_skmeminfo(tb, INET_DIAG_SKMEMINFO);
-	} else if (tb[INET_DIAG_MEMINFO]) {
-		const struct inet_diag_meminfo *minfo
-			= RTA_DATA(tb[INET_DIAG_MEMINFO]);
-		printf(" mem:(r%u,w%u,f%u,t%u)",
-		       minfo->idiag_rmem,
-		       minfo->idiag_wmem,
-		       minfo->idiag_fmem,
-		       minfo->idiag_tmem);
-	}
+	print_skmeminfo(tb, INET_DIAG_SKMEMINFO);
 
 	if (tb[INET_DIAG_INFO]) {
 		struct tcp_info *info;
-- 
2.1.3

^ permalink raw reply related

* [PATCH net-next 4/6] macvlan: advertise link netns via netlink
From: Nicolas Dichtel @ 2015-01-20 14:15 UTC (permalink / raw)
  To: netdev; +Cc: davem, Nicolas Dichtel
In-Reply-To: <1421763347-4354-1-git-send-email-nicolas.dichtel@6wind.com>

Assign rtnl_link_ops->get_link_net() callback so that IFLA_LINK_NETNSID is
added to rtnetlink messages.

Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
 drivers/net/macvlan.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 612e0731142d..1df38bdae2ee 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -1471,11 +1471,17 @@ int macvlan_link_register(struct rtnl_link_ops *ops)
 };
 EXPORT_SYMBOL_GPL(macvlan_link_register);
 
+static struct net *macvlan_get_link_net(const struct net_device *dev)
+{
+	return dev_net(macvlan_dev_real_dev(dev));
+}
+
 static struct rtnl_link_ops macvlan_link_ops = {
 	.kind		= "macvlan",
 	.setup		= macvlan_setup,
 	.newlink	= macvlan_newlink,
 	.dellink	= macvlan_dellink,
+	.get_link_net	= macvlan_get_link_net,
 };
 
 static int macvlan_device_event(struct notifier_block *unused,
-- 
2.2.2

^ permalink raw reply related

* [PATCH net-next 6/6] vxlan: advertise netns of vxlan dev in fdb msg
From: Nicolas Dichtel @ 2015-01-20 14:15 UTC (permalink / raw)
  To: netdev; +Cc: davem, Nicolas Dichtel
In-Reply-To: <1421763347-4354-1-git-send-email-nicolas.dichtel@6wind.com>

Netlink FDB messages are sent in the link netns. The header of these messages
contains the ifindex (ndm_ifindex) of the netdevice, but this ifindex is
unusable in case of x-netns vxlan.
I named the new attribute NDA_NDM_IFINDEX_NETNSID, to avoid confusion with
NDA_IFINDEX.

Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
 drivers/net/vxlan.c            | 5 +++++
 include/uapi/linux/neighbour.h | 1 +
 net/core/net_namespace.c       | 1 +
 3 files changed, 7 insertions(+)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 0346eaa6d236..19d3664ab9dd 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -339,6 +339,11 @@ static int vxlan_fdb_info(struct sk_buff *skb, struct vxlan_dev *vxlan,
 	ndm->ndm_flags = fdb->flags;
 	ndm->ndm_type = RTN_UNICAST;
 
+	if (!net_eq(dev_net(vxlan->dev), vxlan->net) &&
+	    nla_put_s32(skb, NDA_NDM_IFINDEX_NETNSID,
+			peernet2id(vxlan->net, dev_net(vxlan->dev))))
+		goto nla_put_failure;
+
 	if (send_eth && nla_put(skb, NDA_LLADDR, ETH_ALEN, &fdb->eth_addr))
 		goto nla_put_failure;
 
diff --git a/include/uapi/linux/neighbour.h b/include/uapi/linux/neighbour.h
index f3d77f9f1e0b..38f236853cc0 100644
--- a/include/uapi/linux/neighbour.h
+++ b/include/uapi/linux/neighbour.h
@@ -25,6 +25,7 @@ enum {
 	NDA_VNI,
 	NDA_IFINDEX,
 	NDA_MASTER,
+	NDA_NDM_IFINDEX_NETNSID,
 	__NDA_MAX
 };
 
diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
index 9d1a4cac83b6..b7bde551ef76 100644
--- a/net/core/net_namespace.c
+++ b/net/core/net_namespace.c
@@ -202,6 +202,7 @@ int peernet2id(struct net *net, struct net *peer)
 
 	return id >= 0 ? id : NETNSA_NSID_NOT_ASSIGNED;
 }
+EXPORT_SYMBOL(peernet2id);
 
 struct net *get_net_ns_by_id(struct net *net, int id)
 {
-- 
2.2.2

^ permalink raw reply related

* [PATCH net-next 3/6] vlan: advertise link netns via netlink
From: Nicolas Dichtel @ 2015-01-20 14:15 UTC (permalink / raw)
  To: netdev; +Cc: davem, Nicolas Dichtel
In-Reply-To: <1421763347-4354-1-git-send-email-nicolas.dichtel@6wind.com>

Assign rtnl_link_ops->get_link_net() callback so that IFLA_LINK_NETNSID is
added to rtnetlink messages.

Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
 net/8021q/vlan_netlink.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/net/8021q/vlan_netlink.c b/net/8021q/vlan_netlink.c
index 8ac8a5cc2143..c92b52f37d38 100644
--- a/net/8021q/vlan_netlink.c
+++ b/net/8021q/vlan_netlink.c
@@ -238,6 +238,13 @@ nla_put_failure:
 	return -EMSGSIZE;
 }
 
+static struct net *vlan_get_link_net(const struct net_device *dev)
+{
+	struct net_device *real_dev = vlan_dev_priv(dev)->real_dev;
+
+	return dev_net(real_dev);
+}
+
 struct rtnl_link_ops vlan_link_ops __read_mostly = {
 	.kind		= "vlan",
 	.maxtype	= IFLA_VLAN_MAX,
@@ -250,6 +257,7 @@ struct rtnl_link_ops vlan_link_ops __read_mostly = {
 	.dellink	= unregister_vlan_dev,
 	.get_size	= vlan_get_size,
 	.fill_info	= vlan_fill_info,
+	.get_link_net	= vlan_get_link_net,
 };
 
 int __init vlan_netlink_init(void)
-- 
2.2.2

^ permalink raw reply related

* [PATCH net-next 0/6] netns: advertise netns via netlink
From: Nicolas Dichtel @ 2015-01-20 14:15 UTC (permalink / raw)
  To: netdev; +Cc: davem


The first patch of the serie fix a bug of the previous serie (present in
net-next only).
The rest of the serie adds an attribute to advertise the peer netns for
rtnetlink messages where this information is needed by userland to be able to
interpret fully the received message.

 drivers/net/macvlan.c          | 6 ++++++
 drivers/net/veth.c             | 9 +++++++++
 drivers/net/vxlan.c            | 5 +++++
 include/uapi/linux/neighbour.h | 1 +
 net/8021q/vlan_netlink.c       | 8 ++++++++
 net/core/net_namespace.c       | 1 +
 net/core/rtnetlink.c           | 5 ++++-
 net/ipv6/ip6_gre.c             | 1 +
 8 files changed, 35 insertions(+), 1 deletion(-)

Comments are welcome.

Regards,
Nicolas

^ permalink raw reply

* [PATCH net-next 5/6] veth: advertise link netns via netlink
From: Nicolas Dichtel @ 2015-01-20 14:15 UTC (permalink / raw)
  To: netdev; +Cc: davem, Nicolas Dichtel
In-Reply-To: <1421763347-4354-1-git-send-email-nicolas.dichtel@6wind.com>

Assign rtnl_link_ops->get_link_net() callback so that IFLA_LINK_NETNSID is
added to rtnetlink messages.

Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
 drivers/net/veth.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 8ad596573d17..4cca36ebc4fb 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -469,6 +469,14 @@ static const struct nla_policy veth_policy[VETH_INFO_MAX + 1] = {
 	[VETH_INFO_PEER]	= { .len = sizeof(struct ifinfomsg) },
 };
 
+static struct net *veth_get_link_net(const struct net_device *dev)
+{
+	struct veth_priv *priv = netdev_priv(dev);
+	struct net_device *peer = rtnl_dereference(priv->peer);
+
+	return peer ? dev_net(peer) : dev_net(dev);
+}
+
 static struct rtnl_link_ops veth_link_ops = {
 	.kind		= DRV_NAME,
 	.priv_size	= sizeof(struct veth_priv),
@@ -478,6 +486,7 @@ static struct rtnl_link_ops veth_link_ops = {
 	.dellink	= veth_dellink,
 	.policy		= veth_policy,
 	.maxtype	= VETH_INFO_MAX,
+	.get_link_net	= veth_get_link_net,
 };
 
 /*
-- 
2.2.2

^ permalink raw reply related

* [PATCH net-next 2/6] ip6gretap: advertise link netns via netlink
From: Nicolas Dichtel @ 2015-01-20 14:15 UTC (permalink / raw)
  To: netdev; +Cc: davem, Nicolas Dichtel
In-Reply-To: <1421763347-4354-1-git-send-email-nicolas.dichtel@6wind.com>

Assign rtnl_link_ops->get_link_net() callback so that IFLA_LINK_NETNSID is
added to rtnetlink messages.

Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
 net/ipv6/ip6_gre.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
index 9306a5ff9149..6dee2a8ca0a9 100644
--- a/net/ipv6/ip6_gre.c
+++ b/net/ipv6/ip6_gre.c
@@ -1676,6 +1676,7 @@ static struct rtnl_link_ops ip6gre_tap_ops __read_mostly = {
 	.changelink	= ip6gre_changelink,
 	.get_size	= ip6gre_get_size,
 	.fill_info	= ip6gre_fill_info,
+	.get_link_net	= ip6_tnl_get_link_net,
 };
 
 /*
-- 
2.2.2

^ permalink raw reply related

* [PATCH net-next 1/6] rtnl: fix error path when adding an iface with a link net
From: Nicolas Dichtel @ 2015-01-20 14:15 UTC (permalink / raw)
  To: netdev; +Cc: davem, Nicolas Dichtel
In-Reply-To: <1421763347-4354-1-git-send-email-nicolas.dichtel@6wind.com>

If an error occurs when the netdevice is moved to the link netns, a full cleanup
must be done.

Fixes: 317f4810e45e ("rtnl: allow to create device with IFLA_LINK_NETNSID set")
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
 net/core/rtnetlink.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index a12eecc0f976..07447d1665e6 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -2172,8 +2172,11 @@ replay:
 			goto out;
 		}
 
-		if (link_net)
+		if (link_net) {
 			err = dev_change_net_namespace(dev, dest_net, ifname);
+			if (err < 0)
+				unregister_netdevice(dev);
+		}
 out:
 		if (link_net)
 			put_net(link_net);
-- 
2.2.2

^ permalink raw reply related

* Re: [PATCH] ethernet: atheros: Add nss-gmac driver
From: Arnd Bergmann @ 2015-01-20 14:05 UTC (permalink / raw)
  To: wstephen
  Cc: jcliburn, grant.likely, robh+dt, linux-kernel, netdev, devicetree
In-Reply-To: <3ad19b16fe5c58612479f70c052d0045.squirrel@www.codeaurora.org>

On Monday 19 January 2015 21:58:09 wstephen@codeaurora.org wrote:
> > On Thursday 15 January 2015 08:12:51 wstephen@codeaurora.org wrote:
> >
> >> The nss-gmac driver is designed to work standalone or with the nss-drv
> >> driver so the switchable data plane overlay was implemented. When it
> >> worked standalone, the data plane is running on the ARM core as a
> >> standard
> >> networking driver.
> >
> > How do you decide which way it gets used on a particular system?
> >
> 
> By default the GMAC driver uses a native (Host ARM CPU) based data plane. 
> If the configuration loads the offload nss-drv driver, the offload driver
> registers an overlay with the GMAC.

Right. For review purposes, I think it would be helpful to split this
huge patch into several steps then:

- add a base driver
- add the overlay interface
- add the nss driver

Ideally more of them.

> >> The nss-drv driver can take over the data plane and
> >> offload it to the NSS cores. The STMicro stmmac driver does not have
> >> this
> >> kind of overlay design so is not suitable for IPQ806x. This is why we
> >> don't based on the stmmac driver
> >
> > Which kind of offload is implemented specifically? 'data plane' sounds
> > fairly generic and could mean anything, and the code isn't readable enough
> > in its current form for me to find out.
> 
> The Network Subsystem (NSS core) provides a GMAC pass-through until it is
> given a rule to offload work.  Once a rule is given, it is capable of
> performing: bridging, IPv4 NAT/FWD, IPv6 NAT/FWD, IPSec, LAG, and other
> protocols.
> 
> Hope this clarify your question!

Thanks for the description, this sounds very interesting indeed. I do
have more questions though: how do you get the rules into the NSS driver?
Does this get handled transparently by the openvswitch driver or
did you have to add new user interfaces for it?

	Arnd

^ permalink raw reply

* Re: [RFC PATCH net-next] bridge: ability to disable forwarding on a port
From: roopa @ 2015-01-20 13:59 UTC (permalink / raw)
  To: Samudrala, Sridhar
  Cc: Scott Feldman, stephen@networkplumber.org, David S. Miller,
	Jamal Hadi Salim, Jiří Pírko, Arad, Ronen,
	Thomas Graf, john fastabend, vyasevic@redhat.com, Netdev,
	Wilson Kok, Andy Gospodarek
In-Reply-To: <54BDFF20.9020909@intel.com>

On 1/19/15, 11:09 PM, Samudrala, Sridhar wrote:
>
> On 1/19/2015 10:20 PM, roopa wrote:
>> On 1/18/15, 11:37 PM, Scott Feldman wrote:
>>
>> <snip..>
>>>
>>> Not sure. I don't know the use case, but I think I might have heard 
>>> that
>>> there could be a case
>>>   where a switch port could be bridged with a vm's port running on the
>>> switch. (?)
>>>>
>>>> Ignoring the above usecase for a bit. And thinking through this 
>>>> again, It
>>>> appears that this check should
>>>> be only on the ingress port, no ?
>>>>
>>>> If the ingress bridge port is an offloaded port, don't flood or 
>>>> forward
>>>> because hardware has already done it.
>>>> And this is best done with the offload feature flag on the bridge 
>>>> port.
>>> That's assuming hardware did the flood.  Maybe your other option to
>>> mark the skb if already flooded by hw is best.  That's enough info for
>>> the bridge driver to make a decision to flood or not to the other
>>> ports, and it's an implementation decision for the driver/device to do
>>> the flood offload, if desired, and mark the skb if it did.
>>
>> Still thinking we can just use the offload feature flag here. How 
>> about  avoid forwarding only if both src and dst ports have
>> forwarding offloaded/accelerated by a switch asic  ?. That should 
>> cover all cases.
>>>
>>> Btw, you're still saying flood or forward, but in my mind we're
>>> talking about flood only: flood of unknown unicast or flood of
>>> bcast/mcast pkts.  Forwarding would be for known-unicast pkts, which
>>> should even involve the bridge driver since that forwarding is
>>> offloaded to the device.
>>
>> I was also taking into account pkts copied to the CPU due to an acl 
>> rule such as log.
>> These unicast pkts can come to cpu even if it is already forwarded in 
>> hw.
>
> Do you have a switch port netdev that corresponds to CPU port? Or are 
> they seen on the bridge device?
No we dont have a netdev for the CPU port. These show up on the netdev 
corresponding to the ingress front panel port.

^ permalink raw reply

* Re: Bug: mv643xxx fails with highmem
From: Russell King - ARM Linux @ 2015-01-20 13:41 UTC (permalink / raw)
  To: Ezequiel Garcia, David Miller, Linus Torvalds
  Cc: Nimrod Andy, Fabio Estevam, netdev, fugang.duan
In-Reply-To: <20141221165106.GN11285@n2100.arm.linux.org.uk>

Ping.  What's happening on this?

I guess we don't care about regressions in the Linux kernel?

This bug breaks one of my machines, and I'm having to patch around it
by reverting the first two hunks of 69ad0dd7af22, which is only correct
for platforms where dma_unmap_page and dma_unmap_single result in the
same underlying code being used.

I'm not convinced that it's directly caused by 69ad0dd7af22 though.

On Sun, Dec 21, 2014 at 04:51:06PM +0000, Russell King - ARM Linux wrote:
> On Thu, Dec 18, 2014 at 10:13:19AM -0300, Ezequiel Garcia wrote:
> > On 12/17/2014 09:03 PM, Russell King - ARM Linux wrote:
> > > However, exactly how it occurs, I don't know.  My understanding from
> > > reading the various feature flags was that NETIF_F_HIGHDMA was required
> > > for highmem (see illegal_highdma()) so as this isn't set, we shouldn't
> > > be seeing highmem fragments - which is why I asked the question in my
> > > original email.
> > > 
> > > If you want me to revert my fix above, and reproduce again, I can
> > > certainly try that - or put a WARN_ON_ONCE(PageHighMem(this_frag->page.p))
> > > in there, but I seem to remember that it wasn't particularly useful as
> > > the backtrace didn't show where the memory actually came from.
> > > 
> > 
> > No, that's OK. Thanks a lot for all the details. I'll try to come up with a
> > fix soon.
> 
> Well, I decided to add the WARN_ON_ONCE() and re-test.  This I provoked
> by touching etna_viv/src/etnaviv/etna_bo.c, and re-running make (etnaviv
> is on a shared NFS mount.)
> 
> WARNING: CPU: 0 PID: 0 at /home/rmk/git/linux-cubox/drivers/net/ethernet/marvell/mv643xx_eth.c:884 mv643xx_eth_xmit+0x850/0x8dc()
> Modules linked in: bnep rfcomm bluetooth nfsd exportfs ext3 jbd ext2 etnaviv(C)
> snd_soc_spdif_tx orion_wdt snd_soc_kirkwood dove vmeta bmm_dmabuf hwmon snd_soc_kirkwood_spdif
> CPU: 0 PID: 0 Comm: swapper Tainted: G         C     3.18.0+ #1056
> Backtrace:
> [<c0011f54>] (dump_backtrace) from [<c0012228>] (show_stack+0x18/0x1c)
>  r6:00000374 r5:00000009 r4:00000000 r3:00000000
> [<c0012210>] (show_stack) from [<c04992d8>] (dump_stack+0x20/0x28)
> [<c04992b8>] (dump_stack) from [<c0050be4>] (warn_slowpath_common+0x6c/0x8c)
> [<c0050b78>] (warn_slowpath_common) from [<c0050c28>] (warn_slowpath_null+0x24/0x2c)
>  r8:c064ea80 r7:e8a5d880 r6:d00d0d70 r5:e614877c r4:00000001
> [<c0050c04>] (warn_slowpath_null) from [<c02fee9c>] (mv643xx_eth_xmit+0x850/0x8dc)
> [<c02fe64c>] (mv643xx_eth_xmit) from [<c03b94fc>] (dev_hard_start_xmit+0x19c/0x328)
>  r10:c0648054 r9:d0261f60 r8:e6148000 r7:d00ec1c0 r6:c0648040 r5:e623ec00
>  r4:d013c580
> [<c03b9360>] (dev_hard_start_xmit) from [<c03d2728>] (sch_direct_xmit+0x148/0x24c)
>  r10:e623ec00 r9:e63a4580 r8:e6148000 r7:e61f4e00 r6:c063e000 r5:00000000
>  r4:00000000
> [<c03d25e0>] (sch_direct_xmit) from [<c03b985c>] (__dev_queue_xmit+0x1d4/0x590)
>  r10:e623ec00 r9:00000000 r8:00000000 r7:e6148000 r6:c063e000 r5:e61f4e00
>  r4:d0163e70
> [<c03b9688>] (__dev_queue_xmit) from [<c03b9c40>] (dev_queue_xmit+0x14/0x18)
>  r10:c063e000 r9:00000000 r8:00000000 r7:d0163e70 r6:0000000e r5:d00caa00
>  r4:d00caa94
> [<c03b9c2c>] (dev_queue_xmit) from [<c043c58c>] (ip6_finish_output2+0x1a0/0x524)[<c043c3ec>] (ip6_finish_output2) from [<c043e008>] (ip6_output+0xb4/0x174)
>  r10:d0163e70 r9:c063e000 r8:c066f678 r7:00000000 r6:d0163e70 r5:00000000
>  r4:000021c0
> [<c043df54>] (ip6_output) from [<c043c114>] (ip6_xmit+0x278/0x550)
>  r7:00000000 r6:00000001 r5:00000000 r4:001463b6
> [<c043be9c>] (ip6_xmit) from [<c0466fbc>] (inet6_csk_xmit+0x74/0xa8)
>  r10:d0163e70 r9:00000020 r8:d00d2080 r7:d00d25a0 r6:d0163e70 r5:00000000
>  r4:d00d2080
> [<c0466f48>] (inet6_csk_xmit) from [<c03f87ac>] (tcp_transmit_skb+0x494/0x990)
>  r7:e6094100 r6:c0658910 r5:00000020 r4:ffff5165
> [<c03f8318>] (tcp_transmit_skb) from [<c03f9970>] (tcp_write_xmit+0x138/0xc1c)
>  r10:00002178 r9:00000000 r8:00002ca0 r7:00000006 r6:00000594 r5:d0163dc0
>  r4:d00d2080
> [<c03f9838>] (tcp_write_xmit) from [<c03fa4d4>] (__tcp_push_pending_frames+0x38/0x98)
>  r10:00000002 r9:00000078 r8:d03d7600 r7:d00d25a0 r6:d02841c0 r5:e448f778
>  r4:d00d2080
> [<c03fa49c>] (__tcp_push_pending_frames) from [<c03f567c>] (tcp_rcv_established+0x15c/0x600)
>  r4:d00d2080
> [<c03f5520>] (tcp_rcv_established) from [<c0461918>] (tcp_v6_do_rcv+0x2bc/0x46c) r10:00000002 r9:00000078 r8:d03d7600 r7:d00d25a0 r6:00000000 r5:d00d2080
>  r4:d02841c0
> [<c046165c>] (tcp_v6_do_rcv) from [<c0462828>] (tcp_v6_rcv+0x7f8/0x810)
>  r8:00000000 r7:d00d2080 r6:c063e000 r5:c066f678 r4:d02841c0
> [<c0462030>] (tcp_v6_rcv) from [<c043e750>] (ip6_input+0xec/0x424)
>  r10:c066f678 r9:c052a75c r8:d02841c0 r7:c0649720 r6:00000006 r5:e6168400
>  r4:00000006
> [<c043e664>] (ip6_input) from [<c043e100>] (ip6_rcv_finish+0x38/0xa4)
>  r10:e6168400 r9:e6148000 r8:d02841c0 r7:00000001 r6:c066f678 r5:00000000
>  r4:d02841c0 r3:c043e664
> [<c043e0c8>] (ip6_rcv_finish) from [<c043e494>] (ipv6_rcv+0x328/0x4f8)
>  r4:e448f750 r3:00000000
> [<c043e16c>] (ipv6_rcv) from [<c03b43c0>] (__netif_receive_skb_core+0x2fc/0x5d0) r10:d02841c0 r9:c0649050 r8:c06480c4 r7:e6148000 r6:00000000 r5:0000dd86
>  r4:c043e16c
> [<c03b40c4>] (__netif_receive_skb_core) from [<c03b6bac>] (__netif_receive_skb+0x2c/0x88)
>  r10:00000001 r9:d02841c0 r8:e61484e0 r7:e8a5b310 r6:2cc7fffe r5:00000003
>  r4:d02841c0
> [<c03b6b80>] (__netif_receive_skb) from [<c03b6d30>] (netif_receive_skb_internal+0x2c/0x68)
>  r5:00000003 r4:d02841c0
> [<c03b6d04>] (netif_receive_skb_internal) from [<c03b7690>] (napi_gro_receive+0x7c/0xa8)
>  r4:d02841c0
> [<c03b7614>] (napi_gro_receive) from [<c0300738>] (mv643xx_eth_poll+0x58c/0x6ac) r5:e6148000 r4:e614864c
> [<c03001ac>] (mv643xx_eth_poll) from [<c03b7370>] (net_rx_action+0xa4/0x1a8)
>  r10:c0658910 r9:c0675940 r8:c0675940 r7:0000012c r6:00000040 r5:e61485c0
>  r4:c03001ac
> [<c03b72cc>] (net_rx_action) from [<c00533c0>] (__do_softirq+0xf0/0x214)
>  r10:00000003 r9:00000101 r8:c063e000 r7:00000003 r6:c0677480 r5:c067748c
>  r4:00000000
> [<c00532d0>] (__do_softirq) from [<c005377c>] (irq_exit+0xac/0xfc)
>  r10:e6ffcd40 r9:560f5815 r8:00000000 r7:0000001e r6:00000000 r5:00000000
>  r4:c063e000
> [<c00536d0>] (irq_exit) from [<c0085330>] (__handle_domain_irq+0x7c/0xc0)
>  r4:c065ef68 r3:00010001
> [<c00852b4>] (__handle_domain_irq) from [<c000fb0c>] (handle_IRQ+0x24/0x28)
>  r8:00000001 r7:c063ff74 r6:ffffffff r5:600f0013 r4:c0077408 r3:c063ff40
> [<c000fae8>] (handle_IRQ) from [<c0008600>] (dove_legacy_handle_irq+0x34/0x5c)
> [<c00085cc>] (dove_legacy_handle_irq) from [<c0012ce0>] (__irq_svc+0x40/0x74)
> Exception stack(0xc063ff40 to 0xc063ff88)
> ff40: 00000000 0001114c c0658324 c001d940 c063e000 c06470c8 c06759fe c06759fe
> ff60: 00000001 560f5815 e6ffcd40 c063ff9c c063ff88 c063ff88 c000fc38 c0077408
> ff80: 600f0013 ffffffff
> [<c0077354>] (cpu_startup_entry) from [<c04952c4>] (rest_init+0x78/0x90)
>  r7:ffffffff r3:c04b0554
> [<c049524c>] (rest_init) from [<c0609ca8>] (start_kernel+0x34c/0x3b0)
>  r4:c06479b0 r3:00000001
> [<c060995c>] (start_kernel) from [<00008070>] (0x8070)
> 
> Obviously, the useful bit is only down to just above
> __tcp_push_pending_frames(), since that's traces back to the TCP socket's
> send queue.
> 
> If I had to guess where the highmem pages were coming from, I'd suggest
> that it was via the page cache and NFS - maybe something like GCC opens
> a new NFS file, writes to it.  Pages are allocated to it, which happen
> to be allocated from highmem.  NFS eventually sends these pages to the
> NFS server via TCP, which fragments the page and leaves pointers to the
> highmem page in the TCP skbuff.  My NFS mounts are IPv6.
> 
> For Freescale iMX6 FEC, I haven't yet been able to reproduce this there.
> The FEC has tx-checksum-ipv6 and rx-vlan-offload enabled, which the
> mv643xxx driver doesn't have.  The FEC platform has twice the memory of
> the Dove platform (so has much more highmem) so I would've thought it
> would have been easier to reproduce there.  Slightly different kernels
> too - Dove runs 3.18 plus additions, iMX6 is running Linus' tip from
> two days ago plus very similar additions (but same GPU and X server
> code.)
> 
> Other stuff... Dove memory is 0 - 0x3fffffff.  mv643xxx DMA mask
> uninitialised (coherent DMA mask set to 0xffffffff).
> 
> iMX6 is 0x10000000 - 0x8fffffff, with the DMA mask defaulting to
> point at the coherent mask (due to being DT based) which is
> 0xffffffff.
> 
> Here's the diff of the ethtool -k output:
> 
> --- eth0.dove       2014-12-21 16:38:39.000000000 +0000
> +++ eth0.imx6       2014-12-21 16:38:50.792453703 +0000
> @@ -3,7 +3,7 @@
>  tx-checksumming: on
>         tx-checksum-ipv4: on
>         tx-checksum-ip-generic: off [fixed]
> -       tx-checksum-ipv6: off [fixed]
> +       tx-checksum-ipv6: on
>         tx-checksum-fcoe-crc: off [fixed]
>         tx-checksum-sctp: off [fixed]
>  scatter-gather: on
> @@ -17,7 +17,7 @@
>  generic-segmentation-offload: on
>  generic-receive-offload: on
>  large-receive-offload: off [fixed]
> -rx-vlan-offload: off [fixed]
> +rx-vlan-offload: on
>  tx-vlan-offload: off [fixed]
>  ntuple-filters: off [fixed]
>  receive-hashing: off [fixed]
> @@ -32,7 +32,6 @@
>  tx-ipip-segmentation: off [fixed]
>  tx-sit-segmentation: off [fixed]
>  tx-udp_tnl-segmentation: off [fixed]
> -tx-mpls-segmentation: off [fixed]
>  fcoe-mtu: off [fixed]
>  tx-nocache-copy: off
>  loopback: off [fixed]
> 
> Hmm.  I'm now wondering about this:
> 
> static netdev_features_t harmonize_features(struct sk_buff *skb,
>         netdev_features_t features)
> {
> ...
>         if (skb->ip_summed != CHECKSUM_NONE &&
>             !can_checksum_protocol(features, type)) {
>                 features &= ~NETIF_F_ALL_CSUM;
>         } else if (illegal_highdma(skb->dev, skb)) {
>                 features &= ~NETIF_F_SG;
>         }
> 
> For Dove, can_checksum_protocol() would return false for IPv6, which
> would allow the first "if" statement to succeed, hence clearing
> NETIF_F_ALL_CSUM.
> 
> This would prevent the second if() being evaluated - which seems to
> remove the check for any fragments in highmem.  David - shouldn't these
> two checks be independent?
> 
> -- 
> FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
> according to speedtest.net.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply

* Re: [PATCH 11/11] usb: core: fix a race with usb_queue_reset_device()
From: Oliver Neukum @ 2015-01-20 13:48 UTC (permalink / raw)
  To: Olivier Sobrie
  Cc: Jan Dumon, Greg Kroah-Hartman, linux-kernel, linux-usb, netdev
In-Reply-To: <1421756978-4093-12-git-send-email-olivier@sobrie.be>

On Tue, 2015-01-20 at 13:29 +0100, Olivier Sobrie wrote:
> When usb_queue_reset() is called it schedules a work in view of
> resetting the usb interface. When the reset work is running, it
> can be scheduled again (e.g. by the usb disconnect method of
> the driver).
> 
> Consider that the reset work is queued again while the reset work
> is running and that this work leads to a forced unbinding of the
> usb interface (e.g. because a driver is bound to the interface
> and has no pre/post_reset methods - see usb_reset_device()).
> In such condition, usb_unbind_interface() gets called and this
> function calls usb_cancel_queued_reset() which does nothing
> because the flag "reset_running" is set to 1. The second reset
> work that has been scheduled is therefore not cancelled.
> Later, the usb_reset_device() tries to rebind the interface.
> If it fails, then the usb interface context which contain the
> reset work struct is freed and it most likely crash when the
> second reset work tries to be run.
> 
> The following flow shows the problem:
> * usb_queue_reset_device()
> * __usb_queue_reset_device() <- If the reset work is queued after here, then
>     reset_running = 1           it will never be cancelled.
>     usb_reset_device()
>       usb_forced_unbind_intf()
>         usb_driver_release_interface()
>           usb_unbind_interface()
>             driver->disconnect()
>               usb_queue_reset_device() <- second reset

That is the sledgehammer approach. Wouldn't it be better to guarantee
that usb_queue_reset_device() be a nop when reset_running==1 ?

>             usb_cancel_queued_reset() <- does nothing because
>                                          the flag reset_running
>                                          is set
>       usb_unbind_and_rebind_marked_interfaces()
>         usb_rebind_intf()
>           device_attach()
>             driver->probe() <- fails (no more drivers hold a reference to
> 				      the usb interface)
>     reset_running = 0
> * hub_event()
>     usb_disconnect()
>       usb_disable_device()
>         kobject_release()
>           device_release()
>             usb_release_interface()
>               kfree(intf) <- usb interface context is released
>                              while we still have a pending reset
>                              work that should be run
> 
> To avoid this problem, we use a delayed work so that if the reset
> work is currently run, we can avoid further call to
> __usb_queue_reset_device() work by using cancel_delayed_work().
> Unfortunately it increases the size of the usb_interface structure...

	Regards
		Oliver

-- 
Oliver Neukum <oneukum@suse.de>

^ permalink raw reply

* Re: Bug: mv643xxx fails with highmem
From: Ezequiel Garcia @ 2015-01-20 13:42 UTC (permalink / raw)
  To: Russell King - ARM Linux, David Miller, Linus Torvalds
  Cc: Nimrod Andy, Fabio Estevam, netdev, fugang.duan
In-Reply-To: <20150120134104.GA12253@n2100.arm.linux.org.uk>

On 01/20/2015 10:41 AM, Russell King - ARM Linux wrote:
> Ping.  What's happening on this?
> 
> I guess we don't care about regressions in the Linux kernel?
> 

Sorry, a holiday trip got in the middle of this.

I'll try to submit a fix this week.
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

^ 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