Netdev List
 help / color / mirror / Atom feed
* Re: resurrecting tcphealth
From: Piotr Sawuk @ 2012-07-14 23:43 UTC (permalink / raw)
  To: netdev; +Cc: linux-kernel
In-Reply-To: <20120714144800.7f8c97f6@nehalam.linuxnetplumber.net>

On Sa, 14.07.2012, 23:48, Stephen Hemminger wrote:
> Maybe it would be better to use netlink info (for ss command)
> rather than a /proc/net interface.
> See how existing TCP values and MEMINFO are handled.
>

I'm confused, what exactly do you mean?
of course a library-interface might be more interesting than proc/net.
the proc/net/tcphealth functionality probably could be done in userspace,
although I'm wondering how userspace could traverse all tcp_sock structures
for all clients -- but the gain would be a bit more security. and the rest
of the patch still would need to stay in the kernel since that info it
collects would otherwise be lost completely. so I somehow doubt this would
be worth the effort. better would be optional tcphealth disabled by default
for security-reasons. (i.e. if you're a server you don't want to enable it.)

@"David Miller":
sorry, I forgot an important info about this patch, an info that can be
found in the paper I quoted:

'We concentrate on client side metrics to make our results more applicable
to web clients. We chose this strategy to help answer the often asked
question: "Why is my Internet connection so slow?"
...
Since we only have access to the client node and not the server, we must
fi\fnd performance data using only the information coming downstream to us.
Savage showed that the TCP protocol maintains data that we can leverage to
our advantage and we use that idea to monitor per-connection TCP health at
the client. This task is made difficult because of the TCP philosophy of
"smart sender, dumb receiver". There is simply more information about the
connection on the server side than on the client.' [1]

tcp_info is such a server-side diagnosis, clients usually don't send enough
data to make the info therein meaningful for checking the "health" of tcp.
it's all about the senders, and the desktop-pc simply isn't that big of a
tcp-sender to slashdot and co.

@"Eric Dumazet":
the same goes for you too. netstat -s and whatever kind of pooling together
statistics on all internet-connections is useful for a server-admin only,
slowness of particular sites can only be investigated with ping and this
tcphealth patch when you're on client-side.

as for userspace-tools, apart from tcp-health display in each
downloading-progress-bar I can think of cron-jobs which wait for better
tcp-health before spidering some site, or an altered wget in mirror-mode.
the gkrellm-plugin that comes with this patch might count together all the
data of tcphealth, but when a tcp_sock isn't stored anymore that data is
forgotten and you get a concurrent info most likely for only one site, since
the client wont have much connections open and will be closing old
connections frequently.

oh, and again I recommend the really short although outdated thesis

[1] https://sacerdoti.org/tcphealth/tcphealth-paper.pdf

^ permalink raw reply

* [PATCH] b44: add 64 bit stats
From: Kevin Groeneveld @ 2012-07-15  1:51 UTC (permalink / raw)
  To: netdev

From: Kevin Groeneveld <kgroeneveld@gmail.com>

Add support for 64 bit stats to Broadcom b44 ethernet driver.

Signed-off-by: Kevin Groeneveld <kgroeneveld@gmail.com>
---
 drivers/net/ethernet/broadcom/b44.c |   96 ++++++++++++++++++++---------------
 drivers/net/ethernet/broadcom/b44.h |    3 +-
 2 files changed, 58 insertions(+), 41 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/b44.c
b/drivers/net/ethernet/broadcom/b44.c
index 46b8b7d..c799b5c 100644
--- a/drivers/net/ethernet/broadcom/b44.c
+++ b/drivers/net/ethernet/broadcom/b44.c
@@ -483,9 +483,11 @@ out:
 static void b44_stats_update(struct b44 *bp)
 {
 	unsigned long reg;
-	u32 *val;
+	u64 *val;

 	val = &bp->hw_stats.tx_good_octets;
+	u64_stats_update_begin(&bp->hw_stats.syncp);
+
 	for (reg = B44_TX_GOOD_O; reg <= B44_TX_PAUSE; reg += 4UL) {
 		*val++ += br32(bp, reg);
 	}
@@ -496,6 +498,8 @@ static void b44_stats_update(struct b44 *bp)
 	for (reg = B44_RX_GOOD_O; reg <= B44_RX_NPAUSE; reg += 4UL) {
 		*val++ += br32(bp, reg);
 	}
+
+	u64_stats_update_end(&bp->hw_stats.syncp);
 }

 static void b44_link_report(struct b44 *bp)
@@ -1635,44 +1639,49 @@ static int b44_close(struct net_device *dev)
 	return 0;
 }

-static struct net_device_stats *b44_get_stats(struct net_device *dev)
+static struct rtnl_link_stats64 *b44_get_stats64(struct net_device *dev,
+					struct rtnl_link_stats64 *nstat)
 {
 	struct b44 *bp = netdev_priv(dev);
-	struct net_device_stats *nstat = &dev->stats;
 	struct b44_hw_stats *hwstat = &bp->hw_stats;
-
-	/* Convert HW stats into netdevice stats. */
-	nstat->rx_packets = hwstat->rx_pkts;
-	nstat->tx_packets = hwstat->tx_pkts;
-	nstat->rx_bytes   = hwstat->rx_octets;
-	nstat->tx_bytes   = hwstat->tx_octets;
-	nstat->tx_errors  = (hwstat->tx_jabber_pkts +
-			     hwstat->tx_oversize_pkts +
-			     hwstat->tx_underruns +
-			     hwstat->tx_excessive_cols +
-			     hwstat->tx_late_cols);
-	nstat->multicast  = hwstat->tx_multicast_pkts;
-	nstat->collisions = hwstat->tx_total_cols;
-
-	nstat->rx_length_errors = (hwstat->rx_oversize_pkts +
-				   hwstat->rx_undersize);
-	nstat->rx_over_errors   = hwstat->rx_missed_pkts;
-	nstat->rx_frame_errors  = hwstat->rx_align_errs;
-	nstat->rx_crc_errors    = hwstat->rx_crc_errs;
-	nstat->rx_errors        = (hwstat->rx_jabber_pkts +
-				   hwstat->rx_oversize_pkts +
-				   hwstat->rx_missed_pkts +
-				   hwstat->rx_crc_align_errs +
-				   hwstat->rx_undersize +
-				   hwstat->rx_crc_errs +
-				   hwstat->rx_align_errs +
-				   hwstat->rx_symbol_errs);
-
-	nstat->tx_aborted_errors = hwstat->tx_underruns;
+	unsigned int start;
+
+	do {
+		start = u64_stats_fetch_begin(&hwstat->syncp);
+
+		/* Convert HW stats into rtnl_link_stats64 stats. */
+		nstat->rx_packets = hwstat->rx_pkts;
+		nstat->tx_packets = hwstat->tx_pkts;
+		nstat->rx_bytes   = hwstat->rx_octets;
+		nstat->tx_bytes   = hwstat->tx_octets;
+		nstat->tx_errors  = (hwstat->tx_jabber_pkts +
+				     hwstat->tx_oversize_pkts +
+				     hwstat->tx_underruns +
+				     hwstat->tx_excessive_cols +
+				     hwstat->tx_late_cols);
+		nstat->multicast  = hwstat->tx_multicast_pkts;
+		nstat->collisions = hwstat->tx_total_cols;
+
+		nstat->rx_length_errors = (hwstat->rx_oversize_pkts +
+					   hwstat->rx_undersize);
+		nstat->rx_over_errors   = hwstat->rx_missed_pkts;
+		nstat->rx_frame_errors  = hwstat->rx_align_errs;
+		nstat->rx_crc_errors    = hwstat->rx_crc_errs;
+		nstat->rx_errors        = (hwstat->rx_jabber_pkts +
+					   hwstat->rx_oversize_pkts +
+					   hwstat->rx_missed_pkts +
+					   hwstat->rx_crc_align_errs +
+					   hwstat->rx_undersize +
+					   hwstat->rx_crc_errs +
+					   hwstat->rx_align_errs +
+					   hwstat->rx_symbol_errs);
+
+		nstat->tx_aborted_errors = hwstat->tx_underruns;
 #if 0
-	/* Carrier lost counter seems to be broken for some devices */
-	nstat->tx_carrier_errors = hwstat->tx_carrier_lost;
+		/* Carrier lost counter seems to be broken for some devices */
+		nstat->tx_carrier_errors = hwstat->tx_carrier_lost;
 #endif
+	} while (u64_stats_fetch_retry(&hwstat->syncp, start));

 	return nstat;
 }
@@ -1993,17 +2002,24 @@ static void b44_get_ethtool_stats(struct
net_device *dev,
 				  struct ethtool_stats *stats, u64 *data)
 {
 	struct b44 *bp = netdev_priv(dev);
-	u32 *val = &bp->hw_stats.tx_good_octets;
+	struct b44_hw_stats *hwstat = &bp->hw_stats;
+	u64 *data_src, *data_dst;
+	unsigned int start;
 	u32 i;

 	spin_lock_irq(&bp->lock);
-
 	b44_stats_update(bp);
+	spin_unlock_irq(&bp->lock);

-	for (i = 0; i < ARRAY_SIZE(b44_gstrings); i++)
-		*data++ = *val++;
+	do {
+		data_src = &hwstat->tx_good_octets;
+		data_dst = data;
+		start = u64_stats_fetch_begin(&hwstat->syncp);

-	spin_unlock_irq(&bp->lock);
+		for (i = 0; i < ARRAY_SIZE(b44_gstrings); i++)
+			*data_dst++ = *data_src++;
+
+	} while (u64_stats_fetch_retry(&hwstat->syncp, start));
 }

 static void b44_get_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
@@ -2113,7 +2129,7 @@ static const struct net_device_ops b44_netdev_ops = {
 	.ndo_open		= b44_open,
 	.ndo_stop		= b44_close,
 	.ndo_start_xmit		= b44_start_xmit,
-	.ndo_get_stats		= b44_get_stats,
+	.ndo_get_stats64	= b44_get_stats64,
 	.ndo_set_rx_mode	= b44_set_rx_mode,
 	.ndo_set_mac_address	= b44_set_mac_addr,
 	.ndo_validate_addr	= eth_validate_addr,
diff --git a/drivers/net/ethernet/broadcom/b44.h
b/drivers/net/ethernet/broadcom/b44.h
index e1905a4..8993d72 100644
--- a/drivers/net/ethernet/broadcom/b44.h
+++ b/drivers/net/ethernet/broadcom/b44.h
@@ -338,9 +338,10 @@ struct ring_info {
  * the layout
  */
 struct b44_hw_stats {
-#define _B44(x)	u32 x;
+#define _B44(x)	u64 x;
 B44_STAT_REG_DECLARE
 #undef _B44
+	struct u64_stats_sync	syncp;
 };

 struct ssb_device;

^ permalink raw reply related

* RE: 82571EB: Detected Hardware Unit Hang
From: Dave, Tushar N @ 2012-07-15  3:42 UTC (permalink / raw)
  To: Joe Jin
  Cc: e1000-devel@lists.sf.net, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <4FFFA532.1030208@oracle.com>

>-----Original Message-----
>From: Joe Jin [mailto:joe.jin@oracle.com]
>Sent: Thursday, July 12, 2012 9:34 PM
>To: Dave, Tushar N
>Cc: e1000-devel@lists.sf.net; netdev@vger.kernel.org; linux-
>kernel@vger.kernel.org
>Subject: Re: 82571EB: Detected Hardware Unit Hang
>
>On 07/13/12 12:10, Dave, Tushar N wrote:
>>> -----Original Message-----
>>> From: Joe Jin [mailto:joe.jin@oracle.com]
>>> Sent: Thursday, July 12, 2012 4:46 PM
>>> To: Dave, Tushar N
>>> Cc: e1000-devel@lists.sf.net; netdev@vger.kernel.org; linux-
>>> kernel@vger.kernel.org
>>> Subject: Re: 82571EB: Detected Hardware Unit Hang
>>>
>> Thanks for sending full dmesg log. I am still investigating. I think
>this issue can occur if two PCIe link partner *i.e pcie bridge and pcie
>device do not have same max payload size.
>> I need 2 more info.
>> 1) PBA number of the card.
>
>This is a remote server and I could not get this.
>
>> 2) full lspci -vvv output of entire system 'after you have changed max
>payload size to 128'.

Somehow setting max payload to 256 from BIOS does not set this value for all devices. I believe this is a BIOS bug.
All devices in path from root complex to 82571, should have same max payload size otherwise it can cause hang. When you set max payload to 128 from BIOS, all device in path from root complex to 82571 got assigned same max payload size. This resolves the issue.

I hope this helps.

-Tushar

^ permalink raw reply

* Re: 82571EB: Detected Hardware Unit Hang
From: Joe Jin @ 2012-07-15  3:52 UTC (permalink / raw)
  To: Dave, Tushar N
  Cc: netdev@vger.kernel.org, e1000-devel@lists.sf.net,
	linux-kernel@vger.kernel.org
In-Reply-To: <061C8A8601E8EE4CA8D8FD6990CEA891274F2354@ORSMSX102.amr.corp.intel.com>

On 07/15/12 11:42, Dave, Tushar N wrote:
>> -----Original Message-----
>> From: Joe Jin [mailto:joe.jin@oracle.com]
>> Sent: Thursday, July 12, 2012 9:34 PM
>> To: Dave, Tushar N
>> Cc: e1000-devel@lists.sf.net; netdev@vger.kernel.org; linux-
>> kernel@vger.kernel.org
>> Subject: Re: 82571EB: Detected Hardware Unit Hang
>>
>> On 07/13/12 12:10, Dave, Tushar N wrote:
>>>> -----Original Message-----
>>>> From: Joe Jin [mailto:joe.jin@oracle.com]
>>>> Sent: Thursday, July 12, 2012 4:46 PM
>>>> To: Dave, Tushar N
>>>> Cc: e1000-devel@lists.sf.net; netdev@vger.kernel.org; linux-
>>>> kernel@vger.kernel.org
>>>> Subject: Re: 82571EB: Detected Hardware Unit Hang
>>>>
>>> Thanks for sending full dmesg log. I am still investigating. I think
>> this issue can occur if two PCIe link partner *i.e pcie bridge and pcie
>> device do not have same max payload size.
>>> I need 2 more info.
>>> 1) PBA number of the card.
>>
>> This is a remote server and I could not get this.
>>
>>> 2) full lspci -vvv output of entire system 'after you have changed max
>> payload size to 128'.
> 
> Somehow setting max payload to 256 from BIOS does not set this value for all devices. I believe this is a BIOS bug.
> All devices in path from root complex to 82571, should have same max payload size otherwise it can cause hang. When you set max payload to 128 from BIOS, all device in path from root complex to 82571 got assigned same max payload size. This resolves the issue.
> 
> I hope this helps.

Tushar,

Thanks a lot for your help, will send this to hardware engineer.

Regards,
Joe


------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

^ permalink raw reply

* Re: resurrecting tcphealth
From: Eric Dumazet @ 2012-07-15  7:16 UTC (permalink / raw)
  To: Piotr Sawuk; +Cc: netdev, linux-kernel
In-Reply-To: <b93cddfd0a19158ec59823140d1e2075.squirrel@webmail.univie.ac.at>

On Sun, 2012-07-15 at 01:43 +0200, Piotr Sawuk wrote:

> oh, and again I recommend the really short although outdated thesis
> 
> [1] https://sacerdoti.org/tcphealth/tcphealth-paper.pdf

A thesis saying SACK are not useful is highly suspect.

Instead of finding why they behave not so good and fix the bugs, just
say "SACK addition to TCP is not critical"

Really ?

^ permalink raw reply

* Re: [PATCH] b44: add 64 bit stats
From: Eric Dumazet @ 2012-07-15  7:26 UTC (permalink / raw)
  To: Kevin Groeneveld; +Cc: netdev
In-Reply-To: <CABF+-6XaYk8qXnFNXF_+rQ144jyDFgFqDVKsuZv+S_0VZxBAvw@mail.gmail.com>

On Sat, 2012-07-14 at 21:51 -0400, Kevin Groeneveld wrote:
> From: Kevin Groeneveld <kgroeneveld@gmail.com>
> 
> Add support for 64 bit stats to Broadcom b44 ethernet driver.
> 
> Signed-off-by: Kevin Groeneveld <kgroeneveld@gmail.com>
> ---
>  drivers/net/ethernet/broadcom/b44.c |   96 ++++++++++++++++++++---------------
>  drivers/net/ethernet/broadcom/b44.h |    3 +-
>  2 files changed, 58 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/b44.c
> b/drivers/net/ethernet/broadcom/b44.c
> index 46b8b7d..c799b5c 100644
> --- a/drivers/net/ethernet/broadcom/b44.c
> +++ b/drivers/net/ethernet/broadcom/b44.c
> @@ -483,9 +483,11 @@ out:

b44_stats_update() is run from timer, (softirq), check b44_timer()

>  static void b44_stats_update(struct b44 *bp)
>  {
>  	unsigned long reg;
> -	u32 *val;
> +	u64 *val;
> 
>  	val = &bp->hw_stats.tx_good_octets;
> +	u64_stats_update_begin(&bp->hw_stats.syncp);
> +
>  	for (reg = B44_TX_GOOD_O; reg <= B44_TX_PAUSE; reg += 4UL) {
>  		*val++ += br32(bp, reg);
>  	}
> @@ -496,6 +498,8 @@ static void b44_stats_update(struct b44 *bp)
>  	for (reg = B44_RX_GOOD_O; reg <= B44_RX_NPAUSE; reg += 4UL) {
>  		*val++ += br32(bp, reg);
>  	}
> +
> +	u64_stats_update_end(&bp->hw_stats.syncp);
>  }
> 
>  static void b44_link_report(struct b44 *bp)
> @@ -1635,44 +1639,49 @@ static int b44_close(struct net_device *dev)
>  	return 0;
>  }
> 
> -static struct net_device_stats *b44_get_stats(struct net_device *dev)
> +static struct rtnl_link_stats64 *b44_get_stats64(struct net_device *dev,
> +					struct rtnl_link_stats64 *nstat)
>  {
>  
> -	nstat->tx_aborted_errors = hwstat->tx_underruns;
> +	unsigned int start;
> +
> +	do {
> +		start = u64_stats_fetch_begin(&hwstat->syncp);

So you must use u64_stats_fetch_begin_bh()


Because on 32bit, uniprocessor, u64_stats_fetch_begin() only disables
preemption. (there is no seqlock in syncp)

So softirq are allowed to interrupt you and corrupt your stats while you
read them, and you dont notice you have to retry.

> +
> +		/* Convert HW stats into rtnl_link_stats64 stats. */
> +		nstat->rx_packets = hwstat->rx_pkts;
> +		nstat->tx_packets = hwstat->tx_pkts;
> +		nstat->rx_bytes   = hwstat->rx_octets;
> +		nstat->tx_bytes   = hwstat->tx_octets;
> +		nstat->tx_errors  = (hwstat->tx_jabber_pkts +
> +				     hwstat->tx_oversize_pkts +
> +				     hwstat->tx_underruns +
> +				     hwstat->tx_excessive_cols +
> +				     hwstat->tx_late_cols);
> +		nstat->multicast  = hwstat->tx_multicast_pkts;
> +		nstat->collisions = hwstat->tx_total_cols;
> +
> +		nstat->rx_length_errors = (hwstat->rx_oversize_pkts +
> +					   hwstat->rx_undersize);
> +		nstat->rx_over_errors   = hwstat->rx_missed_pkts;
> +		nstat->rx_frame_errors  = hwstat->rx_align_errs;
> +		nstat->rx_crc_errors    = hwstat->rx_crc_errs;
> +		nstat->rx_errors        = (hwstat->rx_jabber_pkts +
> +					   hwstat->rx_oversize_pkts +
> +					   hwstat->rx_missed_pkts +
> +					   hwstat->rx_crc_align_errs +
> +					   hwstat->rx_undersize +
> +					   hwstat->rx_crc_errs +
> +					   hwstat->rx_align_errs +
> +					   hwstat->rx_symbol_errs);
> +
> +		nstat->tx_aborted_errors = hwstat->tx_underruns;
>  #if 0
> -	/* Carrier lost counter seems to be broken for some devices */
> -	nstat->tx_carrier_errors = hwstat->tx_carrier_lost;
> +		/* Carrier lost counter seems to be broken for some devices */
> +		nstat->tx_carrier_errors = hwstat->tx_carrier_lost;
>  #endif
> +	} while (u64_stats_fetch_retry(&hwstat->syncp, start));


u64_stats_fetch_retry_bh()

^ permalink raw reply

* [PATCH] gigaset: silence GCC warning for unused 'format_ie'
From: Paul Bolle @ 2012-07-15  9:11 UTC (permalink / raw)
  To: Tilman Schmidt, Hansjoerg Lipp, Karsten Keil
  Cc: gigaset307x-common, netdev, linux-kernel

Building Gigaset's CAPI support without Gigaset's debugging enabled
triggers this GCC warning:
    'format_ie' defined but not used [-Wunused-function]

Silence this warning by wrapping format_ie() in an "#ifdef
CONFIG_GIGASET_DEBUG" and "#endif" pair.

Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
---
 drivers/isdn/gigaset/capi.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/isdn/gigaset/capi.c b/drivers/isdn/gigaset/capi.c
index 27e4a3e..68452b7 100644
--- a/drivers/isdn/gigaset/capi.c
+++ b/drivers/isdn/gigaset/capi.c
@@ -288,6 +288,7 @@ static inline void dump_rawmsg(enum debuglevel level, const char *tag,
  * format CAPI IE as string
  */
 
+#ifdef CONFIG_GIGASET_DEBUG
 static const char *format_ie(const char *ie)
 {
 	static char result[3 * MAX_FMT_IE_LEN];
@@ -313,6 +314,7 @@ static const char *format_ie(const char *ie)
 	*--pout = 0;
 	return result;
 }
+#endif
 
 /*
  * emit DATA_B3_CONF message
-- 
1.7.7.6

^ permalink raw reply related

* Re: resurrecting tcphealth
From: Piotr Sawuk @ 2012-07-15  9:17 UTC (permalink / raw)
  To: netdev; +Cc: linux-kernel
In-Reply-To: <1342336597.3265.10617.camel@edumazet-glaptop>

On So, 15.07.2012, 09:16, Eric Dumazet wrote:
> On Sun, 2012-07-15 at 01:43 +0200, Piotr Sawuk wrote:
>
>> oh, and again I recommend the really short although outdated thesis
>>
>> [1] https://sacerdoti.org/tcphealth/tcphealth-paper.pdf
>
> A thesis saying SACK are not useful is highly suspect.
>
> Instead of finding why they behave not so good and fix the bugs, just
> say "SACK addition to TCP is not critical"
the actual quotation is "We also found that the number of unnecessary
duplicate packets were quite small potentially indicating that the SACK
addition to TCP is not critical."
>
> Really ?

no, not really. he he actually said that SACK has been made mostly obsolete
by "Linux 2.2 implements fast retransmits for up to two packet gaps, thus
reducing the need for course grained timeouts due to the lack of SACK." and
he was a bit more careful and admitted that further tests with tcphealth are
needed to check if SACK really makes that big a difference. he admitted "It
could be that SACK's advantage lies in other areas such as very large
downloads or when using slow and unreliable network links." all these things
could be checked again nowadays, with larger files available and wlan-users
and higher traffic -- just find something without SACK...

^ permalink raw reply

* Re: resurrecting tcphealth
From: Eric Dumazet @ 2012-07-15  9:53 UTC (permalink / raw)
  To: Piotr Sawuk; +Cc: netdev, linux-kernel
In-Reply-To: <3e426bd471b8a54bfa5320f05e88e635.squirrel@webmail.univie.ac.at>

On Sun, 2012-07-15 at 11:17 +0200, Piotr Sawuk wrote:
> On So, 15.07.2012, 09:16, Eric Dumazet wrote:
> > On Sun, 2012-07-15 at 01:43 +0200, Piotr Sawuk wrote:
> >
> >> oh, and again I recommend the really short although outdated thesis
> >>
> >> [1] https://sacerdoti.org/tcphealth/tcphealth-paper.pdf
> >
> > A thesis saying SACK are not useful is highly suspect.
> >
> > Instead of finding why they behave not so good and fix the bugs, just
> > say "SACK addition to TCP is not critical"
> the actual quotation is "We also found that the number of unnecessary
> duplicate packets were quite small potentially indicating that the SACK
> addition to TCP is not critical."
> >
> > Really ?
> 
> no, not really. he he actually said that SACK has been made mostly obsolete
> by "Linux 2.2 implements fast retransmits for up to two packet gaps, thus
> reducing the need for course grained timeouts due to the lack of SACK." and
> he was a bit more careful and admitted that further tests with tcphealth are
> needed to check if SACK really makes that big a difference. he admitted "It
> could be that SACK's advantage lies in other areas such as very large
> downloads or when using slow and unreliable network links." all these things
> could be checked again nowadays, with larger files available and wlan-users
> and higher traffic -- just find something without SACK...

There are hundred of papers about TCP behavior. Many are very good.

This one seems not the best of them by far, and is based on measures
done on 2001 (!!!), on a single computer (!!!), connected to a
particular ISP (!!!), using a wireless pcmcia network card. (!!!)

At that time, almost no clients were using SACK. Because windows 98/XP
dont negociate SACK by default (you need to tweak registry)

Its like saying ECN is useless : If ECN users are under 1 % of total
number of users, network is still under pressure and ECN benefits cannot
rise because of misbehavior of other flows.

With RTT of 100 ms, SACK are clearly a win for long transferts.

A single drop shall retransmit a single packet instead of ~500 packets.
Only fools could deny this fact. Studying DuplicateAcks to detect
retransmits is clearly wrong.

Really, dont recommend this paper, it contains a lot of false
statements.

One example : "we discovered some surprising things as the high
percentage of lost or reordered packets from supposedly highly reliable
and fast services as Akamai networks". 

I cant believe such nonsense can be written, and recommended.

So you can add more counters to TCP stack because having them is good to
better understand TCP behavior and what can be done to improve it, but
dont base this work on dubious 'tcphealth'.

^ permalink raw reply

* Re: [RFC 1/2] PCI-Express Non-Transparent Bridge Support
From: David Hagood @ 2012-07-15 12:37 UTC (permalink / raw)
  To: Jon Mason; +Cc: Stephen Hemminger, linux-kernel, netdev, linux-pci, Dave Jiang
In-Reply-To: <20120714061953.GD4808@jonmason-lab>

Have you looked at any of the work that the PXI group has done on NTB
support within PXI?
http://www.ni.com/white-paper/12523/en

I was on that working group, and one of the first capabilities I
suggested for it was IP over NTB - I was going to implement this at my
employer, but the project took a different turn and this idea got put on
the back burner. I am glad somebody else has seen fit to take it up.

Also, a stupid question but one I have to ask: have you taken into
account the idea that the CPUs on each side of the NTB might have
different endian-ness? I was looking at a case with a PPC on one side of
the bridge and an X86 on the other.

^ permalink raw reply

* Re: 82571EB: Detected Hardware Unit Hang
From: Henrique de Moraes Holschuh @ 2012-07-15 13:35 UTC (permalink / raw)
  To: Dave, Tushar N
  Cc: Joe Jin, e1000-devel@lists.sf.net, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <061C8A8601E8EE4CA8D8FD6990CEA891274F2354@ORSMSX102.amr.corp.intel.com>

On Sun, 15 Jul 2012, Dave, Tushar N wrote:
> Somehow setting max payload to 256 from BIOS does not set this value for all devices. I believe this is a BIOS bug.

And preferably, Linux should complain about it.  Since we know it is going
to cause problems, and since we know it does happen, we should be raising a
ruckus about it in the kernel log (and probably fixing it to min(path) while
at it)...

Is this something that should be raised as a feature request with the
PCI/PCIe subsystem?

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

^ permalink raw reply

* IPv6 toolkit v1.2
From: Fernando Gont @ 2012-07-15 15:47 UTC (permalink / raw)
  To: netdev

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Folks,

FYI, we've released "IPv6 toolkit v1.2": a set of IPv6 security
assessment tools that were produced as part of a project I carried out
on behalf of the UK CPNI. The toolkit compiles/runs on Linux and *BSD
(and probably on Mac OS, too).

The tarball for version 1.2 of the tool is available at:
http://www.si6networks.com/research/ipv6-toolkit-v1.2.tar.gz>

Additionally, we have a git repository for the tool:
<https://github.com/fgont/ipv6-toolkit.git>

This toolkit can be employed to perform local IPv6 scans, assess a
number of security aspects of IPv6 implementations, such as
predictability of Fragment ID values, etc. It can also be employed to
play with IPv6 address resolution, SLAAC, etc.

Thanks,
- -- 
Fernando Gont
e-mail: fernando@gont.com.ar || fgont@si6networks.com
PGP Fingerprint: 7809 84F5 322E 45C7 F1C9 3945 96EE A9EF D076 FFF1




- -- 
Fernando Gont
SI6 Networks
e-mail: fgont@si6networks.com
PGP Fingerprint: 6666 31C6 D484 63B2 8FB1 E3C4 AE25 0D55 1D4E 7492




-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)

iQIcBAEBAgAGBQJQAuXxAAoJEK4lDVUdTnSSNsQP+gJnHwyo75qkY9CeZFRhNYt1
U1HqMvrBAPPPvO4UT4U611Ek8x1kfJ2jD9n7rpI4PZy04GlLyNbFWOCC6w7AtMHs
c+GfX+rCW6W1W/EYFcrX7rZsZjPMOtgisDShPdMU4hA94blpTULlrry9rfS1CLTm
npNPwqPHQFGSxUK3iNbxJ5G2QXfQ8sSsp+Bi+sk5KUmUmsGVmR/gL5QLzRhbDU58
sA2vkDf0C1cHVRbP/nucyD0wI5x0BRRVLfHAwSm2uOvCGcqrbL9WNH9K35+wHdEa
JFB/Ik01buI7/fci4v1C4QtDheSmcmsR2f2bRRTs7uvNMeMM/nbnHBNxL1t1W8HE
TJWMhXNrEzaqpRNjdmPkpx7XHRqemo+vLvvpABrMfKOf2TqZpi2zuafucuKqbcAB
CwY2QpfEUgQFgCzM9oQvKzn6HCndb39rMrs6VwPuWzg7WCsWGKFj7PIPN2ED4LLI
E3W3xYioRmxCT4HTEunZfigzmbWyZnZuCgA/4gV9PJYKf21SwSkaDTVUzMWHRdSx
wWLlETsv1Me8cAvH09DknImrHR67BIUm3WIlEm8k0IgqFpgNoWtJqfPqIuFSfmOK
idrTKTVlfrtLHpgTiMuKiRPUZzuuxtpOOzCs4o3rfisz/wFPq/7Z7KcQ9vDzdh3u
4Ufow79bF0GF84tL7v8H
=rbry
-----END PGP SIGNATURE-----

^ permalink raw reply

* Re: [PATCH] b44: add 64 bit stats
From: Kevin Groeneveld @ 2012-07-15 17:51 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1342337165.3265.10640.camel@edumazet-glaptop>

Hi Eric,

Thanks for the feedback on my patch.

On Sun, Jul 15, 2012 at 3:26 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> b44_stats_update() is run from timer, (softirq), check b44_timer()

Yes, I knew that.  b44_stats_update() is also called from
b44_get_ethtool_stats().

> So you must use u64_stats_fetch_begin_bh()

That I did not know, although it makes sense after I read more about
it.  I will submit a new patch with this fixed.


Kevin

^ permalink raw reply

* [PATCH v2] b44: add 64 bit stats
From: Kevin Groeneveld @ 2012-07-15 18:00 UTC (permalink / raw)
  To: netdev
In-Reply-To: <CABF+-6VZB=V1j2cAzhNkJozvcVs=VrQGYX_71HiZJ+g6od7qyA@mail.gmail.com>

From: Kevin Groeneveld <kgroeneveld@gmail.com>

Add support for 64 bit stats to Broadcom b44 ethernet driver.

Signed-off-by: Kevin Groeneveld <kgroeneveld@gmail.com>
---
v2: use u64_stats_fetch_begin_bh/u64_stats_fetch_retry_bh instead of
    u64_stats_fetch_begin/u64_stats_fetch_retry as stats update happens in a
    timer interrupt

 drivers/net/ethernet/broadcom/b44.c |   96 ++++++++++++++++++++---------------
 drivers/net/ethernet/broadcom/b44.h |    3 +-
 2 files changed, 58 insertions(+), 41 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/b44.c
b/drivers/net/ethernet/broadcom/b44.c
index 46b8b7d..30ad4ef 100644
--- a/drivers/net/ethernet/broadcom/b44.c
+++ b/drivers/net/ethernet/broadcom/b44.c
@@ -483,9 +483,11 @@ out:
 static void b44_stats_update(struct b44 *bp)
 {
 	unsigned long reg;
-	u32 *val;
+	u64 *val;

 	val = &bp->hw_stats.tx_good_octets;
+	u64_stats_update_begin(&bp->hw_stats.syncp);
+
 	for (reg = B44_TX_GOOD_O; reg <= B44_TX_PAUSE; reg += 4UL) {
 		*val++ += br32(bp, reg);
 	}
@@ -496,6 +498,8 @@ static void b44_stats_update(struct b44 *bp)
 	for (reg = B44_RX_GOOD_O; reg <= B44_RX_NPAUSE; reg += 4UL) {
 		*val++ += br32(bp, reg);
 	}
+
+	u64_stats_update_end(&bp->hw_stats.syncp);
 }

 static void b44_link_report(struct b44 *bp)
@@ -1635,44 +1639,49 @@ static int b44_close(struct net_device *dev)
 	return 0;
 }

-static struct net_device_stats *b44_get_stats(struct net_device *dev)
+static struct rtnl_link_stats64 *b44_get_stats64(struct net_device *dev,
+					struct rtnl_link_stats64 *nstat)
 {
 	struct b44 *bp = netdev_priv(dev);
-	struct net_device_stats *nstat = &dev->stats;
 	struct b44_hw_stats *hwstat = &bp->hw_stats;
-
-	/* Convert HW stats into netdevice stats. */
-	nstat->rx_packets = hwstat->rx_pkts;
-	nstat->tx_packets = hwstat->tx_pkts;
-	nstat->rx_bytes   = hwstat->rx_octets;
-	nstat->tx_bytes   = hwstat->tx_octets;
-	nstat->tx_errors  = (hwstat->tx_jabber_pkts +
-			     hwstat->tx_oversize_pkts +
-			     hwstat->tx_underruns +
-			     hwstat->tx_excessive_cols +
-			     hwstat->tx_late_cols);
-	nstat->multicast  = hwstat->tx_multicast_pkts;
-	nstat->collisions = hwstat->tx_total_cols;
-
-	nstat->rx_length_errors = (hwstat->rx_oversize_pkts +
-				   hwstat->rx_undersize);
-	nstat->rx_over_errors   = hwstat->rx_missed_pkts;
-	nstat->rx_frame_errors  = hwstat->rx_align_errs;
-	nstat->rx_crc_errors    = hwstat->rx_crc_errs;
-	nstat->rx_errors        = (hwstat->rx_jabber_pkts +
-				   hwstat->rx_oversize_pkts +
-				   hwstat->rx_missed_pkts +
-				   hwstat->rx_crc_align_errs +
-				   hwstat->rx_undersize +
-				   hwstat->rx_crc_errs +
-				   hwstat->rx_align_errs +
-				   hwstat->rx_symbol_errs);
-
-	nstat->tx_aborted_errors = hwstat->tx_underruns;
+	unsigned int start;
+
+	do {
+		start = u64_stats_fetch_begin_bh(&hwstat->syncp);
+
+		/* Convert HW stats into rtnl_link_stats64 stats. */
+		nstat->rx_packets = hwstat->rx_pkts;
+		nstat->tx_packets = hwstat->tx_pkts;
+		nstat->rx_bytes   = hwstat->rx_octets;
+		nstat->tx_bytes   = hwstat->tx_octets;
+		nstat->tx_errors  = (hwstat->tx_jabber_pkts +
+				     hwstat->tx_oversize_pkts +
+				     hwstat->tx_underruns +
+				     hwstat->tx_excessive_cols +
+				     hwstat->tx_late_cols);
+		nstat->multicast  = hwstat->tx_multicast_pkts;
+		nstat->collisions = hwstat->tx_total_cols;
+
+		nstat->rx_length_errors = (hwstat->rx_oversize_pkts +
+					   hwstat->rx_undersize);
+		nstat->rx_over_errors   = hwstat->rx_missed_pkts;
+		nstat->rx_frame_errors  = hwstat->rx_align_errs;
+		nstat->rx_crc_errors    = hwstat->rx_crc_errs;
+		nstat->rx_errors        = (hwstat->rx_jabber_pkts +
+					   hwstat->rx_oversize_pkts +
+					   hwstat->rx_missed_pkts +
+					   hwstat->rx_crc_align_errs +
+					   hwstat->rx_undersize +
+					   hwstat->rx_crc_errs +
+					   hwstat->rx_align_errs +
+					   hwstat->rx_symbol_errs);
+
+		nstat->tx_aborted_errors = hwstat->tx_underruns;
 #if 0
-	/* Carrier lost counter seems to be broken for some devices */
-	nstat->tx_carrier_errors = hwstat->tx_carrier_lost;
+		/* Carrier lost counter seems to be broken for some devices */
+		nstat->tx_carrier_errors = hwstat->tx_carrier_lost;
 #endif
+	} while (u64_stats_fetch_retry_bh(&hwstat->syncp, start));

 	return nstat;
 }
@@ -1993,17 +2002,24 @@ static void b44_get_ethtool_stats(struct
net_device *dev,
 				  struct ethtool_stats *stats, u64 *data)
 {
 	struct b44 *bp = netdev_priv(dev);
-	u32 *val = &bp->hw_stats.tx_good_octets;
+	struct b44_hw_stats *hwstat = &bp->hw_stats;
+	u64 *data_src, *data_dst;
+	unsigned int start;
 	u32 i;

 	spin_lock_irq(&bp->lock);
-
 	b44_stats_update(bp);
+	spin_unlock_irq(&bp->lock);

-	for (i = 0; i < ARRAY_SIZE(b44_gstrings); i++)
-		*data++ = *val++;
+	do {
+		data_src = &hwstat->tx_good_octets;
+		data_dst = data;
+		start = u64_stats_fetch_begin_bh(&hwstat->syncp);

-	spin_unlock_irq(&bp->lock);
+		for (i = 0; i < ARRAY_SIZE(b44_gstrings); i++)
+			*data_dst++ = *data_src++;
+
+	} while (u64_stats_fetch_retry_bh(&hwstat->syncp, start));
 }

 static void b44_get_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
@@ -2113,7 +2129,7 @@ static const struct net_device_ops b44_netdev_ops = {
 	.ndo_open		= b44_open,
 	.ndo_stop		= b44_close,
 	.ndo_start_xmit		= b44_start_xmit,
-	.ndo_get_stats		= b44_get_stats,
+	.ndo_get_stats64	= b44_get_stats64,
 	.ndo_set_rx_mode	= b44_set_rx_mode,
 	.ndo_set_mac_address	= b44_set_mac_addr,
 	.ndo_validate_addr	= eth_validate_addr,
diff --git a/drivers/net/ethernet/broadcom/b44.h
b/drivers/net/ethernet/broadcom/b44.h
index e1905a4..8993d72 100644
--- a/drivers/net/ethernet/broadcom/b44.h
+++ b/drivers/net/ethernet/broadcom/b44.h
@@ -338,9 +338,10 @@ struct ring_info {
  * the layout
  */
 struct b44_hw_stats {
-#define _B44(x)	u32 x;
+#define _B44(x)	u64 x;
 B44_STAT_REG_DECLARE
 #undef _B44
+	struct u64_stats_sync	syncp;
 };

 struct ssb_device;

^ permalink raw reply related

* Re: [PATCH v2] b44: add 64 bit stats
From: Eric Dumazet @ 2012-07-15 18:18 UTC (permalink / raw)
  To: Kevin Groeneveld; +Cc: netdev
In-Reply-To: <CABF+-6W0B0wB7hAfSRh83aEb_XPqaJuzA1nKZJygdd9xw-rN1g@mail.gmail.com>

On Sun, 2012-07-15 at 14:00 -0400, Kevin Groeneveld wrote:
> From: Kevin Groeneveld <kgroeneveld@gmail.com>
> 
> Add support for 64 bit stats to Broadcom b44 ethernet driver.
> 
> Signed-off-by: Kevin Groeneveld <kgroeneveld@gmail.com>
> ---
> v2: use u64_stats_fetch_begin_bh/u64_stats_fetch_retry_bh instead of
>     u64_stats_fetch_begin/u64_stats_fetch_retry as stats update happens in a
>     timer interrupt
> 
>  drivers/net/ethernet/broadcom/b44.c |   96 ++++++++++++++++++++---------------
>  drivers/net/ethernet/broadcom/b44.h |    3 +-
>  2 files changed, 58 insertions(+), 41 deletions(-)

Acked-by: Eric Dumazet <edumazet@google.com>

^ permalink raw reply

* [PATCH net] caif: Fix access to freed pernet memory
From: sjur.brandeland @ 2012-07-15 20:10 UTC (permalink / raw)
  To: davem
  Cc: netdev, Eric W. Biederman, sjurbren, Dmitry Tarnyagin,
	Sjur Brændeland

From: Sjur Brændeland <sjur.brandeland@stericsson.com>

unregister_netdevice_notifier() must be called before
unregister_pernet_subsys() to avoid accessing already freed
pernet memory. This fixes the following oops when doing rmmod:

Call Trace:
 [<ffffffffa0f802bd>] caif_device_notify+0x4d/0x5a0 [caif]
 [<ffffffff81552ba9>] unregister_netdevice_notifier+0xb9/0x100
 [<ffffffffa0f86dcc>] caif_device_exit+0x1c/0x250 [caif]
 [<ffffffff810e7734>] sys_delete_module+0x1a4/0x300
 [<ffffffff810da82d>] ? trace_hardirqs_on_caller+0x15d/0x1e0
 [<ffffffff813517de>] ? trace_hardirqs_on_thunk+0x3a/0x3
 [<ffffffff81696bad>] system_call_fastpath+0x1a/0x1f

RIP
 [<ffffffffa0f7f561>] caif_get+0x51/0xb0 [caif]

Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
---

Hi Dave,

Can you please queue up this bugfix as appropriate for -net and -stable?

I guess this bug has been around since introduction of network
name spaces in CAIF, but it became visible after 3.4 and the commit:
"net: In unregister_netdevice_notifier unregister the netdevices." 

Thanks,
Sjur

 net/caif/caif_dev.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/caif/caif_dev.c b/net/caif/caif_dev.c
index 554b312..8c83c17 100644
--- a/net/caif/caif_dev.c
+++ b/net/caif/caif_dev.c
@@ -561,9 +561,9 @@ static int __init caif_device_init(void)
 
 static void __exit caif_device_exit(void)
 {
-	unregister_pernet_subsys(&caif_net_ops);
 	unregister_netdevice_notifier(&caif_device_notifier);
 	dev_remove_pack(&caif_packet_type);
+	unregister_pernet_subsys(&caif_net_ops);
 }
 
 module_init(caif_device_init);
-- 
1.7.5.4

^ permalink raw reply related

* Re: [PATCH 2/6] drivers/net/can/softing/softing_main.c: ensure a consistent return value in error case
From: Kurt Van Dijck @ 2012-07-15 20:21 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Wolfgang Grandegger, kernel-janitors, Marc Kleine-Budde,
	linux-can, netdev, linux-kernel
In-Reply-To: <1342284188-19176-3-git-send-email-Julia.Lawall@lip6.fr>

I see the problem, and the solution. You may add
Acked-by: Kurt Van Dijck <kurt.van.dijck@eia.be>

On Sat, Jul 14, 2012 at 06:43:04PM +0200, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> Typically, the return value desired for the failure of a function with an
> integer return value is a negative integer.  In these cases, the return
> value is sometimes a negative integer and sometimes 0, due to a subsequent
> initialization of the return variable within the loop.
> 
> A simplified version of the semantic match that finds this problem is:
> (http://coccinelle.lip6.fr/)
> 
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> ---
>  drivers/net/can/softing/softing_main.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/can/softing/softing_main.c b/drivers/net/can/softing/softing_main.c
> index a7c77c7..f2a221e 100644
> --- a/drivers/net/can/softing/softing_main.c
> +++ b/drivers/net/can/softing/softing_main.c
> @@ -826,12 +826,12 @@ static __devinit int softing_pdev_probe(struct platform_device *pdev)
>  		goto sysfs_failed;
>  	}
>  
> -	ret = -ENOMEM;
>  	for (j = 0; j < ARRAY_SIZE(card->net); ++j) {
>  		card->net[j] = netdev =
>  			softing_netdev_create(card, card->id.chip[j]);
>  		if (!netdev) {
>  			dev_alert(&pdev->dev, "failed to make can[%i]", j);
> +			ret = -ENOMEM;
>  			goto netdev_failed;
>  		}
>  		priv = netdev_priv(card->net[j]);
> 

^ permalink raw reply

* Re: 3.4.4/amd64 full interrupt hangs under big nfs copies
From: Marc MERLIN @ 2012-07-15 21:59 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David Miller, Larry.Finger, bhutchings, linux-wireless, netdev
In-Reply-To: <20120411052733.GA17352@merlins.org>

On Tue, Apr 10, 2012 at 10:27:33PM -0700, Marc MERLIN wrote:
> On Tue, Apr 10, 2012 at 08:11:03AM +0200, Eric Dumazet wrote:
> > Please try following patch, as it solved the problem for me (no more
> > order-1 allocations in tx path)
> 
> I applied our patch to 3.3.1 and cannot reproduce the problem anymore.
> 
> I'll leave a big wireless copy running overnight just in case, but I think
> you fixed it.

Mmmh, so I'm running 3.4.4 and I had another full machine hang while copying
big files (gigabytes) over wireless via NFS.
The laptop self recovered after 5mn or so (mouse cursor would not even
move) and I was able to kill -9 the process (midnight commander).
mc did not actually stop for another 4mn or so (i.e. it took that long for
the process to come out of kernel hung state), but the machine was usable
during that time.
Note that copying the same data with scp works fine.
NFS mount looks like this:
gargamel:/mnt/dshelf2/ /net/gargamel/mnt/dshelf2 nfs4 rw,nosuid,nodev,relatime,vers=4.0,rsize=524288,wsize=524288,namlen=255,hard,proto=tcp,port=0,timeo=600,retrans=2,sec=sys,clientaddr=192.168.205.7,local_lock=none,addr=192.168.205.3 0 0

I didn't have anything like last time in the kernel logs, and more
annoyingly, ps -elf does not show anything for any process in WCHAN,
making pointing the finger a bit harder (procps-ng 3.3.3 does not show
anything other than '-' in WCHAN for any process with 3.4.4).

My understanding is that user space calling drivers that shut off all
interrupts for extended periods of time (as least I think so since my mouse
cursor would not move), is still a kernel bug.

For what it's worth, copying 1GB of data in lots of small files does not
cause problems, it seems that it's big files that cause a problem since they
likely fill a buffer somewhere while interrupts are disabled.

Do you have an idea of how I can find out where my mc process is stuck in
the kernel?
Should I reproduce with specific sysrq output?

Thanks,
Marc
-- 
"A mouse is a device used to point at the xterm you want to type in" - A.S.R.
Microsoft is to operating systems ....
                                      .... what McDonalds is to gourmet cooking
Home page: http://marc.merlins.org/  

^ permalink raw reply

* Re: resurrecting tcphealth
From: Piotr Sawuk @ 2012-07-15 22:17 UTC (permalink / raw)
  To: netdev; +Cc: linux-kernel
In-Reply-To: <1342346010.3265.10966.camel@edumazet-glaptop>

On So, 15.07.2012, 11:53, Eric Dumazet wrote:
> On Sun, 2012-07-15 at 11:17 +0200, Piotr Sawuk wrote:
>> On So, 15.07.2012, 09:16, Eric Dumazet wrote:
>> > On Sun, 2012-07-15 at 01:43 +0200, Piotr Sawuk wrote:
>> >
>> >> oh, and again I recommend the really short although outdated thesis
>> >>
>> >> [1] https://sacerdoti.org/tcphealth/tcphealth-paper.pdf
>> >
>> > A thesis saying SACK are not useful is highly suspect.
>> >
>> > Instead of finding why they behave not so good and fix the bugs, just
>> > say "SACK addition to TCP is not critical"
>> the actual quotation is "We also found that the number of unnecessary
>> duplicate packets were quite small potentially indicating that the SACK
>> addition to TCP is not critical."
>> >
>> > Really ?
>>
>> no, not really. he he actually said that SACK has been made mostly
>> obsolete
>> by "Linux 2.2 implements fast retransmits for up to two packet gaps, thus
>> reducing the need for course grained timeouts due to the lack of SACK."
>> and
>> he was a bit more careful and admitted that further tests with tcphealth
>> are
>> needed to check if SACK really makes that big a difference. he admitted
>> "It
>> could be that SACK's advantage lies in other areas such as very large
>> downloads or when using slow and unreliable network links." all these
>> things
>> could be checked again nowadays, with larger files available and
>> wlan-users
>> and higher traffic -- just find something without SACK...
>
> There are hundred of papers about TCP behavior. Many are very good.
>
> This one seems not the best of them by far, and is based on measures
> done on 2001 (!!!), on a single computer (!!!), connected to a
> particular ISP (!!!), using a wireless pcmcia network card. (!!!)
>
> At that time, almost no clients were using SACK. Because windows 98/XP
> dont negociate SACK by default (you need to tweak registry)
>
> Its like saying ECN is useless : If ECN users are under 1 % of total
> number of users, network is still under pressure and ECN benefits cannot
> rise because of misbehavior of other flows.

I don't think it's in any way similar, but then you definitely are more
knowledgeable on these matters. but it's quite straight-forward: SACK is
supposed to speed up things by informing when a packet has been received and
doesn't need to be resent. if the client doesn't get duplicate packets then
SACK wont make a difference. tcphealth counts how many packets have been
received correctly two or more times. why would tcphealth be useless for
answering if you'll need SACK?
>
> With RTT of 100 ms, SACK are clearly a win for long transferts.
>
> A single drop shall retransmit a single packet instead of ~500 packets.
> Only fools could deny this fact. Studying DuplicateAcks to detect
> retransmits is clearly wrong.

as far as I understood DuplicateAcks means either the Acks were lost in the
transfer or the same packet has been received twice one after the other.
i.e. duplicate packets received should always be smaller-or-equal than
duplicate acks sent. am I wrong? that's why tcphealth is only interested in
these two values.
>
> Really, dont recommend this paper, it contains a lot of false
> statements.
>
> One example : "we discovered some surprising things as the high
> percentage of lost or reordered packets from supposedly highly reliable
> and fast services as Akamai networks".
>
> I cant believe such nonsense can be written, and recommended.

I agree, it's quite some mickey-mouse thesis. but still a manual for what
tcphealth actually does do!
>
> So you can add more counters to TCP stack because having them is good to
> better understand TCP behavior and what can be done to improve it, but
> dont base this work on dubious 'tcphealth'.

what counters are you thinking of? how do you suggest an ordinary user could
improve the download-speed from sites? imho tcphealth offers a good method:
the more DuplicateAcks you send the less responsive the site is you want to
reach, so try at another time and do some other downloads now...

^ permalink raw reply

* Re: [RFC 1/2] PCI-Express Non-Transparent Bridge Support
From: Jon Mason @ 2012-07-15 23:50 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-kernel, netdev, linux-pci, Dave Jiang
In-Reply-To: <20120714170411.GA25775@kroah.com>

On Sat, Jul 14, 2012 at 10:04:11AM -0700, Greg KH wrote:
> On Fri, Jul 13, 2012 at 02:44:59PM -0700, Jon Mason wrote:
> > The NTB device driver is needed to configure these memory windows, doorbell, and
> > scratch-pad registers as well as use them in such a way as they can be turned
> > into a viable communication channel to the remote system.  ntb_hw.[ch]
> > determines the usage model (NTB to NTB or NTB to Root Port) and abstracts away
> > the underlying hardware to provide access and a common interface to the doorbell
> > registers, scratch pads, and memory windows.  These hardware interfaces are
> > exported so that other, non-mainlined kernel drivers can access these.
> 
> Why would you have non-mainlined drivers?
> 
> Can you submit the drivers at the same time so we see how you are using
> these new interfaces?

There are none at this time.  In the near future, the transport will be modified to use IOAT instead of the CPU copy to improve throughput performance, and it may be beneficial to have that separate.  If you wish for me to remove the hooks until it is necessary for that, then I can.

> 
> > +++ b/drivers/ntb/ntb_hw.c
> > @@ -0,0 +1,1283 @@
> > +/*
> > + * This file is provided under a dual BSD/GPLv2 license.  When using or
> > + *   redistributing this file, you may do so under either license.
> > + *
> > + *   GPL LICENSE SUMMARY
> > + *
> > + *   Copyright(c) 2012 Intel Corporation. All rights reserved.
> > + *
> > + *   This program is free software; you can redistribute it and/or modify
> > + *   it under the terms of version 2 of the GNU General Public License as
> > + *   published by the Free Software Foundation.
> > + *
> > + *   This program is distributed in the hope that it will be useful, but
> > + *   WITHOUT ANY WARRANTY; without even the implied warranty of
> > + *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > + *   General Public License for more details.
> > + *
> > + *   You should have received a copy of the GNU General Public License
> > + *   along with this program; if not, write to the Free Software
> > + *   Foundation, Inc., 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
> > + *   The full GNU General Public License is included in this distribution
> > + *   in the file called LICENSE.GPL.
> 
> You really want to track the office movements of the FSF for the next 40
> years?  You should ask your lawyers if you can remove this paragraph.

Standard boilerplate junk, but it never hurts to ask.
 
> > +/**
> > + * ntb_hw_link_status() - return the hardware link status
> > + * @ndev: pointer to ntb_device instance
> > + *
> > + * Returns true if the hardware is connected to the remote system
> > + *
> > + * RETURNS: true or false based on the hardware link state
> > + */
> > +bool ntb_hw_link_status(struct ntb_device *ndev)
> > +{
> > +	return ndev->link_status == NTB_LINK_UP;
> > +}
> > +EXPORT_SYMBOL(ntb_hw_link_status);
> 
> EXPORT_SYMBOL_GPL() perhaps, for these, and the other symbols you are
> creating?

Will do.

> 
> thanks,
> 
> greg k-h

^ permalink raw reply

* Re: [RFC 1/2] PCI-Express Non-Transparent Bridge Support
From: Greg KH @ 2012-07-15 23:53 UTC (permalink / raw)
  To: Jon Mason; +Cc: linux-kernel, netdev, linux-pci, Dave Jiang
In-Reply-To: <20120715235041.GB7551@jonmason-lab>

On Sun, Jul 15, 2012 at 04:50:41PM -0700, Jon Mason wrote:
> On Sat, Jul 14, 2012 at 10:04:11AM -0700, Greg KH wrote:
> > On Fri, Jul 13, 2012 at 02:44:59PM -0700, Jon Mason wrote:
> > > The NTB device driver is needed to configure these memory windows, doorbell, and
> > > scratch-pad registers as well as use them in such a way as they can be turned
> > > into a viable communication channel to the remote system.  ntb_hw.[ch]
> > > determines the usage model (NTB to NTB or NTB to Root Port) and abstracts away
> > > the underlying hardware to provide access and a common interface to the doorbell
> > > registers, scratch pads, and memory windows.  These hardware interfaces are
> > > exported so that other, non-mainlined kernel drivers can access these.
> > 
> > Why would you have non-mainlined drivers?
> > 
> > Can you submit the drivers at the same time so we see how you are using
> > these new interfaces?
> 
> There are none at this time.  In the near future, the transport will
> be modified to use IOAT instead of the CPU copy to improve throughput
> performance, and it may be beneficial to have that separate.  If you
> wish for me to remove the hooks until it is necessary for that, then I
> can.

Yes, please do so, we don't add apis for things that are not in-kernel
as they almost always need to change once we actually get a user of
them, as you know.

thanks,

greg k-h

^ permalink raw reply

* Re: [RFC 1/2] PCI-Express Non-Transparent Bridge Support
From: Jon Mason @ 2012-07-15 23:55 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-kernel, netdev, linux-pci, Dave Jiang
In-Reply-To: <20120714171015.GB25775@kroah.com>

On Sat, Jul 14, 2012 at 10:10:15AM -0700, Greg KH wrote:
> On Fri, Jul 13, 2012 at 02:44:59PM -0700, Jon Mason wrote:
> > +static int max_num_cbs = 2;
> > +module_param(max_num_cbs, uint, 0644);
> > +MODULE_PARM_DESC(max_num_cbs, "Maximum number of NTB transport connections");
> > +
> > +static bool no_msix;
> > +module_param(no_msix, bool, 0644);
> > +MODULE_PARM_DESC(no_msix, "Do not allow MSI-X interrupts to be selected");
> 
> How would a user, or a distro, know to set these options?  Why are they
> even options at all?

Good question.  There is actually a potential benefit to disabling MSI-X.  The NTB device on one of our platforms only has 3 MSI-X vectors.  In the current driver design, that would limit them to 3 client/virtual devices.  However, there are 15bits in the ISR that can be used for the same purpose.  So, if you disable MSI-X, you can have 15 instead of 3.  

> 
> 
> > +struct ntb_device {
> > +	struct pci_dev *pdev;
> > +	struct msix_entry *msix_entries;
> > +	void __iomem *reg_base;
> > +	struct ntb_mw mw[NTB_NUM_MW];
> > +	struct {
> > +		unsigned int max_spads;
> > +		unsigned int max_db_bits;
> > +		unsigned int msix_cnt;
> > +	} limits;
> > +	struct {
> > +		void __iomem *pdb;
> > +		void __iomem *pdb_mask;
> > +		void __iomem *sdb;
> > +		void __iomem *sbar2_xlat;
> > +		void __iomem *sbar4_xlat;
> > +		void __iomem *spad_write;
> > +		void __iomem *spad_read;
> > +		void __iomem *lnk_cntl;
> > +		void __iomem *lnk_stat;
> > +		void __iomem *spci_cmd;
> > +	} reg_ofs;
> > +	void *ntb_transport;
> > +	void (*event_cb)(void *handle, unsigned int event);
> 
> Shouldn't the event be an enum?

No, that would be too smart.

> 
> > +	struct ntb_db_cb *db_cb;
> > +	unsigned char hw_type;
> > +	unsigned char conn_type;
> > +	unsigned char dev_type;
> > +	unsigned char num_msix;
> > +	unsigned char bits_per_vector;
> > +	unsigned char max_cbs;
> > +	unsigned char link_status;
> > +	struct delayed_work hb_timer;
> > +	unsigned long last_ts;
> > +};
> 
> Why isn't this either a 'struct device' itself, or why isn't the 'struct
> pci_device' embedded within it?  What controls the lifetime of this
> device?  Why doesn't it show up in sysfs?  Don't you want it to show up
> in the global device tree?
> 
> > +static DEFINE_PCI_DEVICE_TABLE(ntb_pci_tbl) = {
> > +	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_B2B_BWD)},
> > +	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_B2B_JSF)},
> > +	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_CLASSIC_JSF)},
> > +	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_RP_JSF)},
> > +	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_RP_SNB)},
> > +	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_B2B_SNB)},
> > +	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_CLASSIC_SNB)},
> > +	{0}
> > +};
> > +MODULE_DEVICE_TABLE(pci, ntb_pci_tbl);
> > +
> > +static struct ntb_device *ntbdev;
> 
> You can really only have just one of these in the whole system?  Is that
> wise?  Why isn't it dynamic and tied to the pci device itself as a
> child?

Good point, I will fix that up.

Thanks for the review!

> 
> thanks,
> 
> greg k-h

^ permalink raw reply

* Re: [RFC 1/2] PCI-Express Non-Transparent Bridge Support
From: Greg KH @ 2012-07-16  0:19 UTC (permalink / raw)
  To: Jon Mason; +Cc: linux-kernel, netdev, linux-pci, Dave Jiang
In-Reply-To: <20120715235548.GC7551@jonmason-lab>

On Sun, Jul 15, 2012 at 04:55:48PM -0700, Jon Mason wrote:
> On Sat, Jul 14, 2012 at 10:10:15AM -0700, Greg KH wrote:
> > On Fri, Jul 13, 2012 at 02:44:59PM -0700, Jon Mason wrote:
> > > +static int max_num_cbs = 2;
> > > +module_param(max_num_cbs, uint, 0644);
> > > +MODULE_PARM_DESC(max_num_cbs, "Maximum number of NTB transport connections");
> > > +
> > > +static bool no_msix;
> > > +module_param(no_msix, bool, 0644);
> > > +MODULE_PARM_DESC(no_msix, "Do not allow MSI-X interrupts to be selected");
> > 
> > How would a user, or a distro, know to set these options?  Why are they
> > even options at all?
> 
> Good question.  There is actually a potential benefit to disabling
> MSI-X.  The NTB device on one of our platforms only has 3 MSI-X
> vectors.  In the current driver design, that would limit them to 3
> client/virtual devices.  However, there are 15bits in the ISR that can
> be used for the same purpose.  So, if you disable MSI-X, you can have
> 15 instead of 3.  

But again, how would a user, or a distro, know to set these?  Where is
the documentation describing it?  Why really have these options at all
and not just fix the platform issues (only 3 MSI-X vectors?  Really?)

thanks,

greg k-h

^ permalink raw reply

* [PATCH net-next] bonding: Support for multi function NIC devices
From: Anirban Chakraborty @ 2012-07-16  1:08 UTC (permalink / raw)
  To: davem; +Cc: netdev, Dept_NX_Linux_NIC_Driver, Anirban Chakraborty

From: Anirban Chakraborty <anirban.chakraborty@qlogic.com>

Add support to disable bonding of interfaces belonging to the same physical port. In
case of SRIOV or NIC partition mode, a single port of the adapter can have multiple
NIC functions. While bonding such interfaces, it is ensured that the NIC functions
belonging to the same physical port are not bonded together.

Signed-off-by: Anirban Chakraborty <anirban.chakraborty@qlogic.com>
---
 Documentation/networking/ifenslave.c |  208 +++++++++++++++++++++++++++++++++-
 1 files changed, 207 insertions(+), 1 deletions(-)

diff --git a/Documentation/networking/ifenslave.c b/Documentation/networking/ifenslave.c
index ac5debb..a0bdab9 100644
--- a/Documentation/networking/ifenslave.c
+++ b/Documentation/networking/ifenslave.c
@@ -92,9 +92,14 @@
  *    - 2003/12/01 - Shmulik Hen <shmulik.hen at intel dot com>
  *	 - Code cleanup and style changes
  *	   set version to 1.1.0
+ *
+ *    - 2012/07/15 - Anirban Chakraborty <anirban.chakraborty at qlogic dot com>
+ *	 - Added support to disable bonding interfaces belonging to the
+ *	   same physical port.
+ *	   set version to 1.1.1
  */
 
-#define APP_VERSION	"1.1.0"
+#define APP_VERSION	"1.1.1"
 #define APP_RELDATE	"December 1, 2003"
 #define APP_NAME	"ifenslave"
 
@@ -111,6 +116,10 @@ static const char *usage_msg =
 "       ifenslave -c   <master-if> <slave-if>\n"
 "       ifenslave --help\n";
 
+static const char *misconfig_msg =
+"Use interfaces from different physical port for an ethernet adapter\n"
+"which has multiple NIC functions belonging to the same physical port\n";
+
 static const char *help_msg =
 "\n"
 "       To create a bond device, simply follow these three steps :\n"
@@ -208,6 +217,18 @@ struct dev_ifr {
 	int req_type;
 };
 
+/* port: physical port
+ * bus: PCI bus no.
+ * ifname: interface name
+ * driver: driver for this device
+ */
+struct dev_prop {
+	u8	port;
+	u16	bus;
+	char	ifname[IFNAMSIZ];
+	char	driver[IFNAMSIZ];
+};
+
 struct dev_ifr master_ifra[] = {
 	{&master_mtu,     "SIOCGIFMTU",     SIOCGIFMTU},
 	{&master_flags,   "SIOCGIFFLAGS",   SIOCGIFFLAGS},
@@ -224,6 +245,10 @@ struct dev_ifr slave_ifra[] = {
 
 static void if_print(char *ifname);
 static int get_drv_info(char *master_ifname);
+static int get_bus_info(struct dev_prop *interface);
+static int get_device_info(int count, struct dev_prop *prop, char *slaves[]);
+static int validate_slaves(char *master, char **spp);
+static int valid_slaves(int count, char *interfaces[]);
 static int get_if_settings(char *ifname, struct dev_ifr ifra[]);
 static int get_slave_flags(char *slave_ifname);
 static int set_master_hwaddr(char *master_ifname, struct sockaddr *hwaddr);
@@ -335,6 +360,15 @@ int main(int argc, char *argv[])
 		goto out;
 	}
 
+	/* validate the slave configuration */
+	if (!opt_d) {
+		res = validate_slaves(master_ifname, spp);
+		if (res) {
+			fprintf(stderr, "%s\n", misconfig_msg);
+			goto out;
+		}
+	}
+
 	slave_ifname = *spp++;
 
 	if (slave_ifname == NULL) {
@@ -643,6 +677,178 @@ out:
 	return 0;
 }
 
+/*
+ * Validate if specified interfaces do not belong to the same physical port
+ */
+static int validate_slaves(char *master, char **spp)
+{
+	int i, count = 0, res = 0;
+	struct ifreq ifr;
+	ifbond bond;
+	ifslave slv;
+	char *bslave;
+	char **slaves, **islaves, **tslaves;
+
+	/* Find a count for existing slave interfaces */
+	memset(&ifr, 0, sizeof(ifr));
+	ifr.ifr_data = (ifbond *)&bond;
+	strncpy(ifr.ifr_name, master, IFNAMSIZ);
+	if (ioctl(skfd, SIOCBONDINFOQUERY, &ifr) < 0) {
+		if (errno == EOPNOTSUPP) {
+			saved_errno = errno;
+			return 1;
+		}
+	}
+	islaves = spp;
+	while (*islaves++ != NULL)
+		count++;
+	if (!count)
+		return 0;
+	count += bond.num_slaves;
+	slaves = malloc(sizeof(char) * count * IFNAMSIZ);
+	if (slaves == NULL)
+		return 1;
+	memset(slaves, 0, (sizeof(char) * count * IFNAMSIZ));
+	tslaves = slaves;
+	/* find new interface names */
+	islaves = spp;
+
+	while (*islaves != NULL)
+		memcpy(slaves++, islaves++, IFNAMSIZ);
+	/* find existing slave interface names */
+	for (i = 0; i < bond.num_slaves; i++) {
+		memset(&slv, 0, sizeof(slv));
+		ifr.ifr_data = (ifslave *)&slv;
+		slv.slave_id = i;
+		if (ioctl(skfd, SIOCBONDSLAVEINFOQUERY, &ifr) < 0) {
+			if (errno == EOPNOTSUPP) {
+				saved_errno = errno;
+				res = 1;
+				goto out;
+			}
+		}
+		bslave = slv.slave_name;
+		memcpy(slaves++, &bslave, IFNAMSIZ);
+	}
+	res = valid_slaves(count, tslaves);
+out:
+	if (tslaves)
+		free(tslaves);
+	return res;
+}
+
+static int get_bus_info(struct dev_prop *interface)
+{
+	struct ifreq ifr;
+	struct ethtool_drvinfo info;
+	char *buf[4], *token, *tmp, *end;
+	int i = 0;
+
+	memset(&ifr, 0, sizeof(ifr));
+	strncpy(ifr.ifr_name, interface->ifname, IFNAMSIZ);
+	ifr.ifr_data = (caddr_t)&info;
+
+	info.cmd = ETHTOOL_GDRVINFO;
+	if (ioctl(skfd, SIOCETHTOOL, &ifr) < 0) {
+		if (errno == EOPNOTSUPP)
+			goto out;
+		saved_errno = errno;
+		v_print("Slave '%s': Error: get bonding info failed %s\n",
+			interface->ifname, strerror(saved_errno));
+		return 1;
+	}
+	memcpy(interface->driver, info.driver, strlen(info.driver) + 1);
+	token = strtok_r(info.bus_info, " :", &tmp);
+	buf[i] =  token;
+	while (token) {
+		token = strtok_r(NULL, " :.", &tmp);
+		buf[++i] = token;
+	}
+	interface->bus = strtoul(buf[1], &end, 16);
+	return 0;
+out:
+	return 1;
+}
+
+static int get_device_info(int count, struct dev_prop *prop, char *slaves[])
+{
+	int ret = -1, i;
+	struct ifreq ifr;
+	struct ethtool_cmd info;
+
+	for (i = 0; i < count; i++) {
+		strncpy(prop[i].ifname, slaves[i], IFNAMSIZ);
+		memset(&ifr, 0, sizeof(ifr));
+		strncpy(ifr.ifr_name, prop[i].ifname, IFNAMSIZ);
+		ifr.ifr_data = (caddr_t)&info;
+
+		info.cmd = ETHTOOL_GSET;
+
+		if (ioctl(skfd, SIOCETHTOOL, &ifr) < 0) {
+			if (errno == EOPNOTSUPP)
+				goto out;
+			saved_errno = errno;
+			v_print("Slave '%s': Error: get bonding info failed"
+				" %s\n", prop[i].ifname,
+				strerror(saved_errno));
+			ret = 1;
+			goto out;
+		}
+		prop[i].port = info.phy_address;
+		ret = get_bus_info(&prop[i]);
+		if (ret)
+			goto out;
+	}
+out:
+	return ret;
+}
+
+/* For a given set of interfaces, find out if they belong to the
+ * same physical port. Return true if two interfaces are found to
+ * be from same physical port, otherwise return false.
+ */
+
+static int valid_slaves(int count, char *slaves[])
+{
+	int i, j, ret = 0;
+	struct dev_prop *ifprop, *searchif;
+	struct dev_prop *prop;
+
+	prop = malloc(count * sizeof(struct dev_prop));
+	if (prop == NULL)
+		return 1;
+
+	memset(prop, 0, sizeof(struct dev_prop) * count);
+	ret = get_device_info(count, prop, slaves);
+	/* Iterate over the array of interfaces and find a match */
+	for (j = 0; j < count; j++) {
+		ifprop = &prop[j];
+		for (i = j + 1; i < count; i++) {
+			searchif = &prop[i];
+			/* Compare driver names */
+			if (!strncmp(ifprop->driver, searchif->driver, IFNAMSIZ)
+				&& !strncmp(ifprop->ifname, searchif->ifname,
+				IFNAMSIZ))
+				continue;
+			/* Compare physical port and bus of the interfaces */
+			if ((searchif->bus == ifprop->bus) &&
+				(searchif->port == ifprop->port)) {
+				ret = 1;
+				fprintf(stderr,
+					"slave interfaces %s and %s "
+					"belong to the same physical "
+					"port of the adapter\n",
+					searchif->ifname, ifprop->ifname);
+				goto out;
+			}
+		}
+	}
+out:
+	free(prop);
+	prop = NULL;
+	return ret;
+}
+
 static int change_active(char *master_ifname, char *slave_ifname)
 {
 	struct ifreq ifr;
-- 
1.7.1

^ permalink raw reply related

* Re: [PATCH] ipvs: fixed sparse warning
From: Simon Horman @ 2012-07-16  1:39 UTC (permalink / raw)
  To: Claudiu Ghioc
  Cc: netdev, davem, netfilter-devel, linux-kernel, wensong, ja, pablo,
	kaber, Claudiu Ghioc
In-Reply-To: <1342198642-22741-1-git-send-email-claudiu.ghioc@gmail.com>

On Fri, Jul 13, 2012 at 07:57:22PM +0300, Claudiu Ghioc wrote:
> Removed the following sparse warnings:
> *	warning: symbol 'ip_vs_control_net_init_sysctl' was not declared. Should
> 	it be static?
> *	warning: symbol 'ip_vs_control_net_cleanup_sysctl' was not
> 	declared. Should it be static?

Thanks.

I wonder if you could post a version of this patch which also fixes the
alternate implementation of ip_vs_control_net_init_sysctl around line 3757 and
the alternate implementation of ip_vs_control_net_cleanup_sysctl that
appears around line 3758.

I believe that you will see sparse warnings for these if
you disable CONFIG_SYSCTL by disabling CONFIG_PROC_SYSCTL.

> 
> Signed-off-by: Claudiu Ghioc <claudiu.ghioc@gmail.com>
> ---
>  net/netfilter/ipvs/ip_vs_ctl.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
> index d43e3c1..3542c6b1 100644
> --- a/net/netfilter/ipvs/ip_vs_ctl.c
> +++ b/net/netfilter/ipvs/ip_vs_ctl.c
> @@ -3674,7 +3674,7 @@ static void ip_vs_genl_unregister(void)
>   * per netns intit/exit func.
>   */
>  #ifdef CONFIG_SYSCTL
> -int __net_init ip_vs_control_net_init_sysctl(struct net *net)
> +static int __net_init ip_vs_control_net_init_sysctl(struct net *net)
>  {
>  	int idx;
>  	struct netns_ipvs *ipvs = net_ipvs(net);
> @@ -3742,7 +3742,7 @@ int __net_init ip_vs_control_net_init_sysctl(struct net *net)
>  	return 0;
>  }
>  
> -void __net_exit ip_vs_control_net_cleanup_sysctl(struct net *net)
> +static void __net_exit ip_vs_control_net_cleanup_sysctl(struct net *net)
>  {
>  	struct netns_ipvs *ipvs = net_ipvs(net);
>  
> -- 
> 1.7.10.4
> 

^ 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