Netdev List
 help / color / mirror / Atom feed
* [PATCH V2 for 3.19 1/7] rtlwifi: Remove logging statement that is no longer needed
From: Larry Finger @ 2015-01-20 17:01 UTC (permalink / raw)
  To: kvalo-sgV2jX0FEOL9JmXXK+q4OQ
  Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA, Larry Finger,
	netdev-u79uwXL29TY76Z2rM5mHXA, Stable
In-Reply-To: <1421773286-1039-1-git-send-email-Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org>

In commit e9538cf4f907 ("rtlwifi: Fix error when accessing unmapped memory
in skb"), a printk was included to indicate that the condition had been
reached. There is now enough evidence from other users that the fix is
working. That logging statement can now be removed.

Signed-off-by: Larry Finger <Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org>
Cc: Stable <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org> [V3.18]
---
 drivers/net/wireless/rtlwifi/pci.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/net/wireless/rtlwifi/pci.c b/drivers/net/wireless/rtlwifi/pci.c
index c70efb9..e25faac 100644
--- a/drivers/net/wireless/rtlwifi/pci.c
+++ b/drivers/net/wireless/rtlwifi/pci.c
@@ -816,11 +816,8 @@ static void _rtl_pci_rx_interrupt(struct ieee80211_hw *hw)
 
 		/* get a new skb - if fail, old one will be reused */
 		new_skb = dev_alloc_skb(rtlpci->rxbuffersize);
-		if (unlikely(!new_skb)) {
-			pr_err("Allocation of new skb failed in %s\n",
-			       __func__);
+		if (unlikely(!new_skb))
 			goto no_new;
-		}
 		if (rtlpriv->use_new_trx_flow) {
 			buffer_desc =
 			  &rtlpci->rx_ring[rxring_idx].buffer_desc
-- 
2.1.2

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH V2 for 3.19 0/7] Fixes for rtl8192ee
From: Larry Finger @ 2015-01-20 17:01 UTC (permalink / raw)
  To: kvalo-sgV2jX0FEOL9JmXXK+q4OQ
  Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA, Larry Finger,
	netdev-u79uwXL29TY76Z2rM5mHXA

Kalle,

This is V2 of the patches for rtl8192ee to be applied to 3.19. They replace
all the patches submitted under the title '[PATCH for 3.19 0/3] rtlwifi:
Various updates/fixes". All are marked for backporting to 3.18.

The first of these removes a logging statement that is no longer needed.

Patches 1-6 are relatively small and should not be a problem for 3.19.
Patch 7 is quite a bit larger, and adds two new routines to detect
DMA stalls. I will understand if you want to defer that to -next; however,
it does fix a serious problem.

Larry
---
Larry Finger (2):
  rtlwifi: Remove logging statement that is no longer needed
  rtlwifi: rtl8192ee: Fix problems with calculating free space in FIFO

Troy Tan (5):
  rtlwifi: rtl8192ee: Fix handling of new style descriptors
  rtlwifi: rtl8192ee: Fix adhoc fail
  rtlwifi: rtl8192ee: Fix TX hang due to failure to update TX write
    point
  rtlwifi: rtl8192ee: Fix parsing of received packet
  rtlwifi: rtl8192ee: Fix DMA stalls

 drivers/net/wireless/rtlwifi/pci.c           |  36 ++++--
 drivers/net/wireless/rtlwifi/pci.h           |   7 ++
 drivers/net/wireless/rtlwifi/rtl8192ee/fw.c  |   6 +-
 drivers/net/wireless/rtlwifi/rtl8192ee/hw.c  | 166 ++++++++++++++++++++++-----
 drivers/net/wireless/rtlwifi/rtl8192ee/reg.h |   2 +
 drivers/net/wireless/rtlwifi/rtl8192ee/sw.c  |   3 +-
 drivers/net/wireless/rtlwifi/rtl8192ee/trx.c |  36 +++---
 drivers/net/wireless/rtlwifi/rtl8192ee/trx.h |   4 +-
 drivers/net/wireless/rtlwifi/wifi.h          |   1 +
 9 files changed, 192 insertions(+), 69 deletions(-)

-- 
2.1.2

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH net-next] rhashtable: rhashtable_remove() must unlink in both tbl and future_tbl
From: Thomas Graf @ 2015-01-20 16:58 UTC (permalink / raw)
  To: davem, Ying Xue
  Cc: richard.alpe@ericsson.com >> Richard Alpe, Netdev,
	tipc-discussion
In-Reply-To: <54BDC30D.5000606@windriver.com>

As removals can occur during resizes, entries may be referred to from
both tbl and future_tbl when the removal is requested. Therefore
rhashtable_remove() must unlink the entry in both tables if this is
the case. The existing code did search both tables but stopped when it
hit the first match.

Failing to do so resulted in use after remove.

Fixes: 97defe1 ("rhashtable: Per bucket locks & deferred expansion/shrinking")
Reported-by: Ying Xue <ying.xue@windriver.com>
Signed-off-by: Thomas Graf <tgraf@suug.ch>
---

Ying: This should fix the panic that was at the end of your TIPC
      related boot log. I'm still working on the use after free.

 lib/rhashtable.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index a4449c4..b1aa10e 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -592,6 +592,7 @@ bool rhashtable_remove(struct rhashtable *ht, struct rhash_head *obj)
 	struct rhash_head *he;
 	spinlock_t *lock;
 	unsigned int hash;
+	bool ret = false;
 
 	rcu_read_lock();
 	tbl = rht_dereference_rcu(ht->tbl, ht);
@@ -609,17 +610,16 @@ restart:
 		}
 
 		rcu_assign_pointer(*pprev, obj->next);
-		atomic_dec(&ht->nelems);
-
-		spin_unlock_bh(lock);
-
-		rhashtable_wakeup_worker(ht);
-
-		rcu_read_unlock();
 
-		return true;
+		ret = true;
+		break;
 	}
 
+	/* The entry may be linked in either 'tbl', 'future_tbl', or both.
+	 * 'future_tbl' only exists for a short period of time during
+	 * resizing. Thus traversing both is fine and the added cost is
+	 * very rare.
+	 */
 	if (tbl != rht_dereference_rcu(ht->future_tbl, ht)) {
 		spin_unlock_bh(lock);
 
@@ -632,9 +632,15 @@ restart:
 	}
 
 	spin_unlock_bh(lock);
+
+	if (ret) {
+		atomic_dec(&ht->nelems);
+		rhashtable_wakeup_worker(ht);
+	}
+
 	rcu_read_unlock();
 
-	return false;
+	return ret;
 }
 EXPORT_SYMBOL_GPL(rhashtable_remove);
 
-- 
1.9.3

^ permalink raw reply related

* Re: [PATCH v2 net-next] net: ipv6: Add sysctl entry to disable MTU updates from RA
From: Daniel Borkmann @ 2015-01-20 16:56 UTC (permalink / raw)
  To: Harout Hedeshian; +Cc: netdev
In-Reply-To: <006301d034d0$f9e10520$eda30f60$@codeaurora.org>

On 01/20/2015 05:48 PM, Harout Hedeshian wrote:
...
> I will upload v3 with the following changes:
>   - Fix tab indentation in documentation
>   - Remove changes from sysctl_binary.c and sysctl.h
>   - Fixed netlink dumps

Sounds good, thanks!

^ permalink raw reply

* RE: [PATCH v2 net-next] net: ipv6: Add sysctl entry to disable MTU updates from RA
From: Harout Hedeshian @ 2015-01-20 16:48 UTC (permalink / raw)
  To: 'Daniel Borkmann'; +Cc: netdev
In-Reply-To: <54BD8CB0.8050800@redhat.com>

> > diff --git a/include/uapi/linux/ipv6.h b/include/uapi/linux/ipv6.h
> > index 73cb02d..437a6a4 100644
> > --- a/include/uapi/linux/ipv6.h
> > +++ b/include/uapi/linux/ipv6.h
> > @@ -169,6 +169,7 @@ enum {
> >   	DEVCONF_SUPPRESS_FRAG_NDISC,
> >   	DEVCONF_ACCEPT_RA_FROM_LOCAL,
> >   	DEVCONF_USE_OPTIMISTIC,
> > +	DEVCONF_ACCEPT_RA_MTU,
> >   	DEVCONF_MAX
> >   };
> 
> You also need a corresponding ipv6_store_devconf() entry, otherwise
> netlink dumps will always see this setting as disabled.

I added the following:
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index cdd70ed..5743293 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -4378,6 +4378,7 @@ static inline void ipv6_store_devconf(struct
ipv6_devconf *cnf,
        array[DEVCONF_NDISC_NOTIFY] = cnf->ndisc_notify;
        array[DEVCONF_SUPPRESS_FRAG_NDISC] = cnf->suppress_frag_ndisc;
        array[DEVCONF_ACCEPT_RA_FROM_LOCAL] = cnf->accept_ra_from_local;
+       array[DEVCONF_ACCEPT_RA_MTU] = cnf->accept_ra_mtu;
 }

I will upload v3 with the following changes:
 - Fix tab indentation in documentation
 - Remove changes from sysctl_binary.c and sysctl.h
 - Fixed netlink dumps

-- Sanity test results below --

As a quick test, I wrote a very simple utility to dump out the raw contents
of a RTM_NEWLINK event and tested it out on a UML instance by assigning some
easily identifiable (but meaningless) value:

root@debian-wheezy-template:/mnt/host# ./a.out &
root@debian-wheezy-template:/mnt/host# cd /proc/sys/net/ipv6/conf/eth0
root@debian-wheezy-template:/proc/sys/net/ipv6/conf/eth0# echo 90 >
accept_ra_mtu #should print the letter 'Z'
root@debian-wheezy-template:/proc/sys/net/ipv6/conf/eth0# ifconfig eth0 up
&& ifconfig eth0 down
Read 1152 bytes from the socket
           0  1  2  3  4  5  6  7  8  9  A  B  C  D  E  F
...<truncated>...
00000020| 09 00 03 00 65 74 68 30 00 00 00 00 08 00 0D 00 | ....eth0........
...<truncated>...
00000300| 00 00 00 00 00 00 00 00 5A 00 00 00 24 01 03 00 | ........Z...$...
root@debian-wheezy-template:/proc/sys/net/ipv6/conf/eth0# echo 89 >
accept_ra_mtu # should print the letter 'Y'
root@debian-wheezy-template:/proc/sys/net/ipv6/conf/eth0# ifconfig eth0 up
&& ifconfig eth0 down
Read 1152 bytes from the socket
           0  1  2  3  4  5  6  7  8  9  A  B  C  D  E  F
...<truncated>...
00000020| 09 00 03 00 65 74 68 30 00 00 00 00 08 00 0D 00 | ....eth0........
...<truncated>...
00000300| 00 00 00 00 00 00 00 00 59 00 00 00 24 01 03 00 | ........Y...$...


Thanks,
Harout

--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux
Foundation Collaborative Project

^ permalink raw reply related

* Re: [PATCH 11/11] usb: core: fix a race with usb_queue_reset_device()
From: Olivier Sobrie @ 2015-01-20 16:30 UTC (permalink / raw)
  To: Alan Stern; +Cc: Jan Dumon, Greg Kroah-Hartman, linux-kernel, linux-usb, netdev
In-Reply-To: <Pine.LNX.4.44L0.1501201022360.1150-100000@iolanthe.rowland.org>

On Tue, Jan 20, 2015 at 10:26:30AM -0500, Alan Stern wrote:
> On Tue, 20 Jan 2015, 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.
> 
> There was an earlier patch posted for testing (no results yet)  
> affecting this same region of code:
> 
> 	http://marc.info/?l=linux-usb&m=142064533924019&w=2
> 
> It should fix the problem described here, because (among other things) 
> it adds usb_get/put_intf calls to the delayed-reset routines.

Ok sorry I didn't see that patch. It looks indeed that it should
solve my problem.
I'll give it a try tomorrow.

Thank you,

-- 
Olivier

^ permalink raw reply

* Re: [PATCH net-next v8 2/4] net: netcp: Add Keystone NetCP core ethernet driver
From: Murali Karicheri @ 2015-01-20 15:55 UTC (permalink / raw)
  To: Paul Bolle
  Cc: Wingman Kwok, Valentin Rothberg, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1421741935.9655.73.camel@x220>

On 01/20/2015 03:18 AM, Paul Bolle wrote:
> Murali, Wingman,
>
> On Thu, 2015-01-15 at 19:12 -0500, Murali Karicheri wrote:
>> The network coprocessor (NetCP) is a hardware accelerator available in
>> Keystone SoCs that processes Ethernet packets. NetCP consists of following
>> hardware components
>>
>>   1 Gigabit Ethernet (GbE) subsystem with a Ethernet switch sub-module to
>>     send and receive packets.
>>   2 Packet Accelerator (PA) module to perform packet classification
>>     operations such as header matching, and packet modification operations
>>     such as checksum generation.
>>   3 Security Accelerator(SA) capable of performing IPSec operations on
>>     ingress/egress packets.
>>   4 An optional 10 Gigabit Ethernet Subsystem (XGbE) which includes a
>>     3-port Ethernet switch sub-module capable of 10Gb/s and 1Gb/s rates
>>     per Ethernet port.
>>   5 Packet DMA and Queue Management Subsystem (QMSS) to enqueue and dequeue
>>     packets and DMA the packets between memory and NetCP hardware components
>>     described above.
>>
>> NetCP core driver make use of the Keystone Navigator driver API to allocate
>> DMA channel for the Ethenet device and to handle packet queue/de-queue,
>> Please refer API's in include/linux/soc/ti/knav_dma.h and
>> drivers/soc/ti/knav_qmss.h for details.
>>
>> NetCP driver consists of NetCP core driver and at a minimum Gigabit
>> Ethernet (GBE) module (1) driver to implement the Network device function.
>> Other modules (2,3) can be optionally added to achieve supported hardware
>> acceleration function. The initial version of the driver include NetCP
>> core driver and GBE driver modules.
>>
>> Please refer Documentation/devicetree/bindings/net/keystone-netcp.txt
>> for design of the driver.
>>
>>   Cc: David Miller<davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
>>   Cc: Rob Herring<robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>>   Cc: Grant Likely<grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>   Cc: Santosh Shilimkar<santosh.shilimkar-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>>   Cc: Pawel Moll<pawel.moll-5wv7dgnIgG8@public.gmane.org>
>>   Cc: Mark Rutland<mark.rutland-5wv7dgnIgG8@public.gmane.org>
>>   Cc: Ian Campbell<ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>
>>   Cc: Kumar Gala<galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>>
>> Signed-off-by: Murali Karicheri<m-karicheri2-l0cyMroinI0@public.gmane.org>
>> Signed-off-by: Wingman Kwok<w-kwok2-l0cyMroinI0@public.gmane.org>
>
> This patch is included as commit 84640e27f230 ("net: netcp: Add Keystone
> NetCP core ethernet driver") in today's linux-next. I noticed because a
> script I run on linux-next spotted a problem in it.
>
>> ---
>>   MAINTAINERS                          |    7 +
>>   drivers/net/ethernet/ti/Kconfig      |   11 +
>>   drivers/net/ethernet/ti/Makefile     |    3 +
>>   drivers/net/ethernet/ti/netcp.h      |  229 ++++
>>   drivers/net/ethernet/ti/netcp_core.c | 2141 ++++++++++++++++++++++++++++++++++
>>   5 files changed, 2391 insertions(+)
>>   create mode 100644 drivers/net/ethernet/ti/netcp.h
>>   create mode 100644 drivers/net/ethernet/ti/netcp_core.c
>>
>> [...]
>> diff --git a/drivers/net/ethernet/ti/Kconfig b/drivers/net/ethernet/ti/Kconfig
>> index 605dd90..e11bcfa 100644
>> --- a/drivers/net/ethernet/ti/Kconfig
>> +++ b/drivers/net/ethernet/ti/Kconfig
>> @@ -73,12 +73,23 @@ config TI_CPSW
>>   config TI_CPTS
>>   	boolean "TI Common Platform Time Sync (CPTS) Support"
>>   	depends on TI_CPSW
>> +	depends on TI_CPSW || TI_KEYSTONE_NET
>
> You probably meant to add
>      || TI_KEYSTONE_NETCP
>
> here. Ie, add CP. But as this slipped through testing it _might_ not be
> needed at all.
Paul,

I will get back to you on this today.

Thanks

Murali
>
>>   	select PTP_1588_CLOCK
>>   	---help---
>>   	  This driver supports the Common Platform Time Sync unit of
>>   	  the CPSW Ethernet Switch. The unit can time stamp PTP UDP/IPv4
>>   	  and Layer 2 packets, and the driver offers a PTP Hardware Clock.
>>
>> +config TI_KEYSTONE_NETCP
>> +	tristate "TI Keystone NETCP Ethernet subsystem Support"
>> +	depends on OF
>> +	depends on KEYSTONE_NAVIGATOR_DMA&&  KEYSTONE_NAVIGATOR_QMSS
>> +	---help---
>> +	  This driver supports TI's Keystone NETCP Ethernet subsystem.
>> +
>> +	  To compile this driver as a module, choose M here: the module
>> +	  will be called keystone_netcp.
>> +
>>   config TLAN
>>   	tristate "TI ThunderLAN support"
>>   	depends on (PCI || EISA)
>
> Thanks,
>
>
> Paul Bolle
>


-- 
Murali Karicheri
Linux Kernel, Texas Instruments
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH net-next v8 0/4] net: Add Keystone NetCP ethernet driver support
From: Murali Karicheri @ 2015-01-20 15:53 UTC (permalink / raw)
  To: David Miller
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150119.151106.822126941934010309.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>

On 01/19/2015 03:11 PM, David Miller wrote:
> From: Murali Karicheri<m-karicheri2-l0cyMroinI0@public.gmane.org>
> Date: Thu, 15 Jan 2015 19:10:03 -0500
>
>> The Network Coprocessor (NetCP) is a hardware accelerator that processes
>> Ethernet packets. NetCP has a gigabit Ethernet (GbE) subsystem with a ethernet
>> switch sub-module to send and receive packets. NetCP also includes a packet
>> accelerator (PA) module to perform packet classification operations such as
>> header matching, and packet modification operations such as checksum
>> generation. NetCP can also optionally include a Security Accelerator(SA)
>> capable of performing IPSec operations on ingress/egress packets.
>>
>> Keystone SoC's also have a 10 Gigabit Ethernet Subsystem (XGbE) which
>> includes a 3-port Ethernet switch sub-module capable of 10Gb/s and
>> 1Gb/s rates per Ethernet port.
>>
>> Both GBE and XGBE network processors supported using common driver. It
>> is also designed to handle future variants of NetCP.
>
> Series applied to net-next, thanks.
David,

Thanks a lot for applying this series. This helps us move forward to 
work on the next set of patches.

Thanks
-- 
Murali Karicheri
Linux Kernel, Texas Instruments
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH net 2/2] bnx2x: fix napi poll return value for repoll
From: Eric Dumazet @ 2015-01-20 15:44 UTC (permalink / raw)
  To: Govindarajulu Varadarajan
  Cc: Willem de Bruijn, davem, netdev, ariel.elior, ssujith, benve
In-Reply-To: <alpine.LNX.2.03.1501202027430.31736@ws.cisco>

On Tue, 2015-01-20 at 20:54 +0530, Govindarajulu Varadarajan wrote:
> On Tue, 20 Jan 2015, Eric Dumazet wrote:
> 
> > 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.
> >
> 
> This is based on f41281d02f8b94e136f78cb1b6a5d78182c222bd &
> 9dfa9a27b620640322588df399eb8f624b48d877
> 
> I do not know about bnx2x, but in enic driver, when busy_poll is enables
> rq clean is not happening. This is because, in napi_poll() when
> work_done < budget, we do not repoll. At this point, enic has disables rq intr
> and has not called napi_complete. Driver assumes that napi will repoll.
> Which does not happen.
> 
> Lot of drivers I have checked return full budget if they want to repoll.
> eg. mlx4_en_poll_rx_cq()

I was referring to the "workdone > budget" condition.

But yes, the fix about busy poll seems needed.

^ permalink raw reply

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

On 01/20/15 at 03:21pm, Patrick McHardy wrote:
> I think its preferrable to make the need to handle NETLINK_F_DUMP_INTR
> as noticable as possible and not hide it. Silent failure is the worst
> kind of failure.

I agree to that. The point here is to avoid unnecessary use of
NETLINK_F_DUMP_INTR if all entries fit into a single message buffer.
 
> Yeah. The point could be made that this is a regression though. We didn't
> require userspace to deal with interruptions before, and the behaviour
> was well defined and acceptable for most cases, its not anymore.
> 
> So I think it should be handled by the kernel, without changes to
> userspace.

nl_table_lock was released between individual messages just like
ht->mutex is released with this change.

What changed is that inserts and removal can now occur *while* the
message is being constructed whereas previously they could only
occur between message construction periods. In either case, the dump
would end up missing entries or showing duplicates.

^ permalink raw reply

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

On Tue, 20 Jan 2015, Eric Dumazet wrote:

> 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()
>

Yes, I missed that one. This change here is not needed.

Thanks

^ permalink raw reply

* Re: [PATCH 11/11] usb: core: fix a race with usb_queue_reset_device()
From: Alan Stern @ 2015-01-20 15:26 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, 20 Jan 2015, 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.

There was an earlier patch posted for testing (no results yet)  
affecting this same region of code:

	http://marc.info/?l=linux-usb&m=142064533924019&w=2

It should fix the problem described here, because (among other things) 
it adds usb_get/put_intf calls to the delayed-reset routines.

Alan Stern

^ permalink raw reply

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

On Tue, 20 Jan 2015, Eric Dumazet wrote:

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

This is based on f41281d02f8b94e136f78cb1b6a5d78182c222bd &
9dfa9a27b620640322588df399eb8f624b48d877

I do not know about bnx2x, but in enic driver, when busy_poll is enables
rq clean is not happening. This is because, in napi_poll() when
work_done < budget, we do not repoll. At this point, enic has disables rq intr
and has not called napi_complete. Driver assumes that napi will repoll.
Which does not happen.

Lot of drivers I have checked return full budget if they want to repoll.
eg. mlx4_en_poll_rx_cq()

^ permalink raw reply

* Re: [PATCH 3/3] netlink: Lock out table resizes while dumping Netlink sockets
From: Patrick McHardy @ 2015-01-20 15:21 UTC (permalink / raw)
  To: Thomas Graf; +Cc: davem, herbert, paulmck, ying.xue, netdev, netfilter-devel
In-Reply-To: <20150120145551.GH20315@casper.infradead.org>

On 20.01, Thomas Graf wrote:
> 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.

I think its preferrable to make the need to handle NETLINK_F_DUMP_INTR
as noticable as possible and not hide it. Silent failure is the worst
kind of failure.

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

Yeah. The point could be made that this is a regression though. We didn't
require userspace to deal with interruptions before, and the behaviour
was well defined and acceptable for most cases, its not anymore.

So I think it should be handled by the kernel, without changes to
userspace.

^ permalink raw reply

* 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


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