* Re: [PATCH RESEND net] net/mlx4_en: Limit the RFS filter IDs to be < RPS_NO_FILTER
From: David Miller @ 2012-07-25 22:23 UTC (permalink / raw)
To: ogerlitz; +Cc: bhutchings, netdev, oren, amirv
In-Reply-To: <50100B03.2020205@mellanox.com>
From: Or Gerlitz <ogerlitz@mellanox.com>
Date: Wed, 25 Jul 2012 18:04:35 +0300
> On 25/07/2012 17:57, Ben Hutchings wrote:
>>> @@ -77,12 +77,8 @@ int mlx4_en_activate_cq(struct mlx4_en_priv *priv,
>>> struct mlx4_en_cq *cq,
>>> struct mlx4_en_dev *mdev = priv->mdev;
>>> int err = 0;
>>> char name[25];
>>> - struct cpu_rmap *rmap =
>>> -#ifdef CONFIG_RFS_ACCEL
>>> - priv->dev->rx_cpu_rmap;
>>> -#else
>>> - NULL;
>>> -#endif
>>> + struct cpu_rmap *rmap = IS_ENABLED(CONFIG_RFS_ACCEL) ?
>>> + priv->dev->rx_cpu_rmap : NULL;
>>
>> This is a separate change.
>
> OK, will send two patches
This change breaks the build.
You can't do this check at run-time, because the reason you need to
check CONFIG_RFS_ACCEL is because if that's disabled then the netdev
structure doesn't even have the ->rx_cpu_rmap member.
^ permalink raw reply
* Re: [PATCH] be2net: Missing byteswap in be_get_fw_log_level causes oops on PowerPC
From: David Miller @ 2012-07-25 22:24 UTC (permalink / raw)
To: anton; +Cc: sathya.perla, subbu.seetharaman, ajit.khaparde, netdev
In-Reply-To: <20120725110525.0468f754@kryten>
From: Anton Blanchard <anton@samba.org>
Date: Wed, 25 Jul 2012 11:05:25 +1000
>
> We are seeing an oops in be_get_fw_log_level on ppc64 where we walk
> off the end of memory.
>
> commit 941a77d582c8 (be2net: Fix to allow get/set of debug levels in
> the firmware.) requires byteswapping of num_modes and num_modules.
>
> Cc: stable@vger.kernel.org # 3.5+
> Signed-off-by: Anton Blanchard <anton@samba.org>
Applied.
^ permalink raw reply
* Re: [patch 0/3] s390: bug fixes for net
From: David Miller @ 2012-07-25 22:24 UTC (permalink / raw)
To: frank.blaschka; +Cc: netdev, linux-s390
In-Reply-To: <20120725083426.704834478@de.ibm.com>
From: frank.blaschka@de.ibm.com
Date: Wed, 25 Jul 2012 10:34:26 +0200
> here are some bug fixes for net.
All applied, thanks.
^ permalink raw reply
* Re: [PATCH v0 net-next 1/1] net/pch_gpe: Cannot disable ethernet autonegation
From: David Miller @ 2012-07-25 22:25 UTC (permalink / raw)
To: w90p710; +Cc: netdev
In-Reply-To: <1343218439-5403-1-git-send-email-w90p710@gmail.com>
From: w90p710@gmail.com
Date: Wed, 25 Jul 2012 20:13:59 +0800
> From: Wei Yang <w90p710@gmail.com>
>
> When attempting to disable ethernet autonegation via ethtool,
> the pch_gpe driver will set software reset bit of PHY chip, But
> control register of PHY chip of FRI2 will reenable ethernet autonegation.
>
> Signed-off-by: Wei Yang <w90p710@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH] ipv4: rt_cache_valid must check expired routes
From: David Miller @ 2012-07-25 22:25 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1343229083.2626.11177.camel@edumazet-glaptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 25 Jul 2012 17:11:23 +0200
> From: Eric Dumazet <edumazet@google.com>
>
> commit d2d68ba9fe8 (ipv4: Cache input routes in fib_info nexthops.)
> introduced rt_cache_valid() helper. It unfortunately doesn't check if
> route is expired before caching it.
>
> I noticed sk_setup_caps() was constantly called on a tcp workload.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied, thanks.
> ---
> This patch is bringed by an European guy, but its still July after
> all ;)
:-)
^ permalink raw reply
* Re: [PATCH RESEND net] net/mlx4_en: Limit the RFS filter IDs to be < RPS_NO_FILTER
From: Ben Hutchings @ 2012-07-25 22:29 UTC (permalink / raw)
To: David Miller; +Cc: ogerlitz, netdev, oren, amirv
In-Reply-To: <20120725.152337.155627167530086828.davem@davemloft.net>
On Wed, 2012-07-25 at 15:23 -0700, David Miller wrote:
> From: Or Gerlitz <ogerlitz@mellanox.com>
> Date: Wed, 25 Jul 2012 18:04:35 +0300
>
> > On 25/07/2012 17:57, Ben Hutchings wrote:
> >>> @@ -77,12 +77,8 @@ int mlx4_en_activate_cq(struct mlx4_en_priv *priv,
> >>> struct mlx4_en_cq *cq,
> >>> struct mlx4_en_dev *mdev = priv->mdev;
> >>> int err = 0;
> >>> char name[25];
> >>> - struct cpu_rmap *rmap =
> >>> -#ifdef CONFIG_RFS_ACCEL
> >>> - priv->dev->rx_cpu_rmap;
> >>> -#else
> >>> - NULL;
> >>> -#endif
> >>> + struct cpu_rmap *rmap = IS_ENABLED(CONFIG_RFS_ACCEL) ?
> >>> + priv->dev->rx_cpu_rmap : NULL;
> >>
> >> This is a separate change.
> >
> > OK, will send two patches
>
> This change breaks the build.
>
> You can't do this check at run-time, because the reason you need to
> check CONFIG_RFS_ACCEL is because if that's disabled then the netdev
> structure doesn't even have the ->rx_cpu_rmap member.
Yes, sorry for suggesting that, Or.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [RFC NET-NEXT PATCH 4/6] e1000: configure and read MDI settings
From: Ben Hutchings @ 2012-07-25 22:41 UTC (permalink / raw)
To: Jesse Brandeburg; +Cc: netdev
In-Reply-To: <20120725180020.9995.32923.stgit@jbrandeb-snb.jf.intel.com>
On Wed, 2012-07-25 at 11:00 -0700, Jesse Brandeburg wrote:
> this is the implementation in e1000 to allow ethtool to force
> MDI state, allowing users to work around some improperly
> behaving switches.
>
> forcing in this driver is for now only allowed when auto-neg is enabled.
>
> to use must have the matching version of ethtool app that supports
> this functionality.
[...]
> --- a/drivers/net/ethernet/intel/e1000/e1000_main.c
> +++ b/drivers/net/ethernet/intel/e1000/e1000_main.c
> @@ -4939,6 +4939,10 @@ int e1000_set_spd_dplx(struct e1000_adapter *adapter, u32 spd, u8 dplx)
> default:
> goto err_inval;
> }
> +
> + /* clear MDI, MDI(-X) override is only allowed when autoneg enabled */
> + hw->mdix = AUTO_ALL_MODES;
You will translate AUTO_ALL_MODES to ETH_TP_MDI_AUTO in
e1000_get_settings, and then treat that as an error in
e1000_set_settings if hw->media_type != e1000_media_type_copper. So,
does this assignment need to be conditional on hw->media_type ==
e1000_media_type_copper?
Ben.
> return 0;
>
> err_inval:
>
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [RFC NET-NEXT PATCH 5/6] e1000e: implement MDI/MDI-X control
From: Ben Hutchings @ 2012-07-25 22:43 UTC (permalink / raw)
To: Jesse Brandeburg; +Cc: netdev
In-Reply-To: <20120725180025.9995.5613.stgit@jbrandeb-snb.jf.intel.com>
On Wed, 2012-07-25 at 11:00 -0700, Jesse Brandeburg wrote:
> some users report issues with link failing when connected to certain
> switches. This gives the user the ability to control the MDI state
> from the driver, allowing users to work around some improperly
> behaving switches.
>
> forcing in this driver is for now only allowed when auto-neg is
> enabled.
[...]
> --- a/drivers/net/ethernet/intel/e1000e/ethtool.c
> +++ b/drivers/net/ethernet/intel/e1000e/ethtool.c
[...]
> @@ -241,6 +246,10 @@ static int e1000_set_spd_dplx(struct e1000_adapter *adapter, u32 spd, u8 dplx)
> default:
> goto err_inval;
> }
> +
> + /* clear MDI, MDI(-X) override is only allowed when autoneg enabled */
> + adapter->hw.phy.mdix = AUTO_ALL_MODES;
> +
[...]
I have the same question about this assignment as in e1000.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [RFC NET-NEXT PATCH 0/6] implement MDI-X set support in Intel drivers
From: Ben Hutchings @ 2012-07-25 22:48 UTC (permalink / raw)
To: Jesse Brandeburg; +Cc: netdev
In-Reply-To: <20120725175430.9995.11370.stgit@jbrandeb-snb.jf.intel.com>
On Wed, 2012-07-25 at 10:59 -0700, Jesse Brandeburg wrote:
> This is an update (v2) to the patches sent previously.
>
> This series implements MDI-X set support in the Intel gigabit ethernet
> drivers. It specifically allows a user to control the MDI state,
> in order for them to work around issues with broken switches.
>
> The only change to this series is updated descriptions and an
> addition of reporting the current mdix_ctrl value in the
> "get_settings" functions, which allows the new extended reporting
> in the ethtool app's MDI-X: line
>
> sent as RFC for now to get feedback while internal testing
> completes. (and net-next is closed)
[...]
This all looks sane.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH net-next,1/2] hyperv: Add a check for ring_size value
From: David Miller @ 2012-07-25 22:48 UTC (permalink / raw)
To: haiyangz; +Cc: netdev, kys, olaf, linux-kernel, devel
In-Reply-To: <1343239722-21328-1-git-send-email-haiyangz@microsoft.com>
From: Haiyang Zhang <haiyangz@microsoft.com>
Date: Wed, 25 Jul 2012 11:08:41 -0700
> It prevents ring_size being set to a too small value.
>
> Reported-by: Jason Wang <jasowang@redhat.com>
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> Reviewed-by: K. Y. Srinivasan <kys@microsoft.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next,2/2] hyperv: Add error handling to rndis_filter_device_add()
From: David Miller @ 2012-07-25 22:49 UTC (permalink / raw)
To: haiyangz; +Cc: netdev, olaf, linux-kernel, devel
In-Reply-To: <1343239722-21328-2-git-send-email-haiyangz@microsoft.com>
From: Haiyang Zhang <haiyangz@microsoft.com>
Date: Wed, 25 Jul 2012 11:08:42 -0700
> Reported-by: Jason Wang <jasowang@redhat.com>
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> Reviewed-by: K. Y. Srinivasan <kys@microsoft.com>
Applied.
^ permalink raw reply
* Re: [RFC ETHTOOL PATCH 2/2] ethtool: allow setting MDI-X state
From: Ben Hutchings @ 2012-07-25 22:59 UTC (permalink / raw)
To: Jesse Brandeburg; +Cc: netdev
In-Reply-To: <20120725175308.9839.50343.stgit@jbrandeb-snb.jf.intel.com>
On Wed, 2012-07-25 at 10:53 -0700, Jesse Brandeburg wrote:
> A bit ago ethtool added support for reading MDI-X state, this
> patch finishes the implementation, adding the complementary write
> command.
>
> Add support to ethtool for controlling the MDI-X (crossover)
> state of a network port. Most adapters correctly negotiate
> MDI-X, but some ill-behaved switches have trouble and end up
> picking the wrong MDI setting, which results in complete loss of
> link. Usually this error condition can be observed when multiple
> ethtool -r ethX are required before link is achieved.
>
> This patch allows the user to override the normal "auto" setting
> and force the crossover state to on or off.
>
> The set will fail if the driver doesn't support the get, as
> suggested by Ben Hutchings.
>
> # ./ethtool -s p1p1 mdix off
> setting MDI not supported
>
> In addition the do_gset output was changed slightly to report the
> value set by the user (when the driver supports the set)
>
> old:
> MDI-X: on
>
> new:
> MDI-X: on (auto)
> or
> MDI-X: on (forced)
>
> usage is ethtool -s eth0 mdix [auto|on|off]
[...]
> --- a/ethtool.c
> +++ b/ethtool.c
> @@ -18,6 +18,8 @@
> * Rx Network Flow Control configuration support <santwona.behera@sun.com>
> * Various features by Ben Hutchings <bhutchings@solarflare.com>;
> * Copyright 2009, 2010 Solarflare Communications
> + * MDI-X set support by Jesse Brandeburg <jesse.brandeburg@intel.com>
> + * Copyright 2012 Intel Corporation
> *
> * TODO:
> * * show settings for all devices
> @@ -559,13 +561,29 @@ static int dump_ecmd(struct ethtool_cmd *ep)
> fprintf(stdout, " MDI-X: ");
> switch (ep->eth_tp_mdix) {
> case ETH_TP_MDI:
> - fprintf(stdout, "off\n");
> + fprintf(stdout, "off");
> break;
> case ETH_TP_MDI_X:
> - fprintf(stdout, "on\n");
> + fprintf(stdout, "on");
> break;
> default:
> - fprintf(stdout, "Unknown\n");
> + fprintf(stdout, "Unknown");
> + break;
> + }
> + switch (ep->eth_tp_mdix_ctrl) {
> + case ETH_TP_MDI:
> + case ETH_TP_MDI_X:
> + /* forced via sset */
> + fprintf(stdout, " (forced)\n");
[...]
How about when you have a forced mode but the driver reports eth_tp_mdix
= ETH_TP_MDI_INVALID because the link is down? This is going to result
in:
MDI-X: Unknown (forced)
which makes no sense at all. So I think that we have to do something
like:
if (ep->eth_tp_mdix_ctrl == ETH_TP_MDI) {
fprintf(stdout, "off (forced)\n");
} else if (ep->eth_tp_mdix_ctrl == ETH_TP_MDI_X) {
fprintf(stdout, "on (forced)\n");
} else {
switch (ep->eth_tp_mdix) {
...
}
if (ep->eth_tp_mdix_ctrl == ETH_TP_MDI_AUTO)
fprintf(stdout, " (auto)");
fprintf(stdout, "\n");
}
Or else we require that when the mode is forced then drivers report that
as the current status even if the link is down.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH 00/16] Remove the ipv4 routing cache
From: Alexander Duyck @ 2012-07-25 23:02 UTC (permalink / raw)
To: David Miller, Eric Dumazet; +Cc: netdev
In-Reply-To: <20120720.142502.1144557295933737451.davem@davemloft.net>
On Fri, Jul 20, 2012 at 2:25 PM, David Miller <davem@davemloft.net> wrote:
>
> [ Ok I'm going to be a little bit cranky, and I think I deserve it.
>
> I'm basically not going to go through the multi-hour rebase and
> retest process again, as it's hit the point of diminishing returns
> as NOBODY is giving me test results but I can guarentee that
> EVERYONE will bitch and complain when I push this into net-next and
> it breaks their favorite feature. If you can't be bothered to test
> these changes, I'm honestly going to tell people to take a hike and
> fix it themselves. I simply don't care if you don't care enough to
> test changes of this magnitude to make sure your favorite setup
> still works.
>
> To say that I'm disappointed with the amount of testing feedback
> after posting more than a dozen iterations of this delicate patch
> set would be an understatement. I can think of only one person who
> actually tested one iteration of these patches and gave feedback.
>
> And meanwhile I've personally reviewed, tested, and signed off on
> everyone else's work WITHOUT DELAY during this entire process.
>
> I've pulled 25 hour long hacking shifts to make that a reality, so
> that my routing cache removal work absolutely would not impact or
> delay the patch submissions of any other networking developer. And
> I can't even get a handful of testers with some feedback? You
> really have to be kidding me.. ]
Sorry for not responding sooner but I have been on vacation for the
last few days.
I had been testing the patches over the last couple of weeks but I
didn't really feel like I could provide any input of value since I
don't have a strong understanding of the routing stack internals, and
because my test case tends to focus on small packet routing with a
very artificial work flow. I saw an overall drop in performance. I
had attributed to the fact that with so few flows I was exploiting the
routing cache to it's maximum potential, and had not explored it much
further.
My test consists of a SmartBits w/ a 10Gb/s port connected back to
back with one port on an 82599 adapter. I have the SmartBits
generating up to 16 64byte packet flows, each flow is filtered through
an ntuple filter to a specific queue, and each queue is pinned to a
specific CPU. The flows all have a unique source address but the same
destination address. The port doing the routing has two subnets. We
receive packets on the first subnet and then transmit them back out on
the second. I have set-up a static ARP entry for the destination
address in order to avoid the need for ARP address translation since
we are sending such a heavy packet load. My kernel config is stripped
down and does not include netfilter support.
Since your patches are in I have started to re-run my tests. I am
seeing a significant drop in throughput with 8 flows which I expected,
however it looks like one of the biggest issues I am seeing is that
the dst_hold and dst_release calls seem to be causing some serious
cache thrash. I was at 12.5Mpps w/ 8 flows before the patches, after
your patches it drops to 8.3Mpps. If I increase the number of queues
I am using to 16 the throughput drops off to something like 3.3Mpps.
Prior to your patches being applied the top 3 CPU consumers were
ixgbe_poll at around 10%, ixgbe_xmit_frame_ring at around 5%, and
__netif_receive_skb at around 5%. Below is the latest perf results
for 8 flows/queues after your patches:
14.52% [k] __write_lock_failed
10.68% [k] ip_route_input (75% of hits on dst_hold call)
10.18% [k] fib_table_lookup
6.04% [k] ixgbe_poll
5.80% [k] dst_release
4.14% [k] __netif_receive_skb
3.55% [k] _raw_spin_lock
2.84% [k] ip_forward
2.58% [k] ixgbe_xmit_frame_ring
I am also seeing routing fail periodically. I will be moving at rates
listed above and suddenly drop to single digits packets per second.
When this occurs the trace completely changes and __write_lock_failed
jumps to over 90% of the CPU cycles. It seems to occur more often if
I increase the number of CPUs in use while routing. Below is the call
graph I recorded for the function from perf to show the function calls
that are leading to the issue:
14.52% [k] __write_lock_failed
|
|--99.92%-- _raw_write_lock_bh
| __neigh_event_send
| neigh_resolve_output
| ip_finish_output
| ip_output
| ip_forward
| ip_rcv
| __netif_receive_skb
| netif_receive_skb
| napi_skb_finish
| napi_gro_receive
| ixgbe_poll
| net_rx_action
| __do_softirq
| run_ksoftirqd
| kthread
| kernel_thread_helper
--0.08%-- [...]
I am trying to figure out what can be done, but as I said I am not
that familiar with the internals of the IP routing stack itself. If
you need more data let me know and I can see about performing whatever
test, or altering my configuration as needed.
Thanks,
Alex
^ permalink raw reply
* Re: [PATCH 00/16] Remove the ipv4 routing cache
From: David Miller @ 2012-07-25 23:17 UTC (permalink / raw)
To: alexander.duyck; +Cc: eric.dumazet, netdev
In-Reply-To: <CAKgT0UdcqE52WFCT-t2om0SN3Yq5GqiwN+JbwCcnBv4LDOB6BQ@mail.gmail.com>
From: Alexander Duyck <alexander.duyck@gmail.com>
Date: Wed, 25 Jul 2012 16:02:45 -0700
> Since your patches are in I have started to re-run my tests. I am
> seeing a significant drop in throughput with 8 flows which I expected,
> however it looks like one of the biggest issues I am seeing is that
> the dst_hold and dst_release calls seem to be causing some serious
> cache thrash. I was at 12.5Mpps w/ 8 flows before the patches, after
> your patches it drops to 8.3Mpps.
Yes, this is something we knew would start happening.
One idea is to make cached dsts be per-cpu in the nexthops.
> I am also seeing routing fail periodically.
Every 30 seconds by chance? :-)
> I will be moving at rates listed above and suddenly drop to single
> digits packets per second. When this occurs the trace completely
> changes and __write_lock_failed jumps to over 90% of the CPU cycles.
It's probably happening when the nexthop ARP entry expires.
^ permalink raw reply
* Re: [PATCH 00/16] Remove the ipv4 routing cache
From: David Miller @ 2012-07-25 23:39 UTC (permalink / raw)
To: alexander.duyck; +Cc: eric.dumazet, netdev
In-Reply-To: <20120725.161732.91008692477078715.davem@davemloft.net>
From: David Miller <davem@davemloft.net>
Date: Wed, 25 Jul 2012 16:17:32 -0700 (PDT)
> From: Alexander Duyck <alexander.duyck@gmail.com>
> Date: Wed, 25 Jul 2012 16:02:45 -0700
>
>> Since your patches are in I have started to re-run my tests. I am
>> seeing a significant drop in throughput with 8 flows which I expected,
>> however it looks like one of the biggest issues I am seeing is that
>> the dst_hold and dst_release calls seem to be causing some serious
>> cache thrash. I was at 12.5Mpps w/ 8 flows before the patches, after
>> your patches it drops to 8.3Mpps.
>
> Yes, this is something we knew would start happening.
>
> One idea is to make cached dsts be per-cpu in the nexthops.
Actually I think what really kills your case is the removal of the
noref path for route lookups. I'll work on a patch to restore that
in the case where we use cached routes from the FIB nexthops.
^ permalink raw reply
* [PATCH] dynamic_debug: Restore dev_dbg/netdev_dbg functionality, reduce stack use
From: Joe Perches @ 2012-07-26 0:10 UTC (permalink / raw)
To: Greg Kroah-Hartman, David S. Miller, Jason Baron
Cc: Kay Sievers, Jim Cromie, Andrew Morton, linux-kernel, netdev
commit c4e00daaa9 ("driver-core: extend dev_printk() to pass structured data")
changed __dev_printk and broke dynamic-debug's ability to control the
dynamic prefix of dev_dbg(dev,..).
dynamic_emit_prefix() adds "[tid] module:func:line:" to the output and
those additions got lost.
In addition, the current dynamic debug code uses up to 3 recursion
levels via %pV. This can consume quite a bit of stack. Directly
call printk_emit to reduce the recursion by one depth.
These changes include:
o Remove KERN_DEBUG from dynamic_emit_prefix
o Create and use function create_syslog_header to format the syslog
header for printk_emit uses.
o Call create_syslog_header and neaten __dev_printk
o Call create_syslog_header and printk_emit from dynamic_dev_dbg
o Call create_syslog_header and printk_emit from dynamic_netdev_dbg
o Make __dev_printk and __netdev_printk static not global
o Remove include header declarations of __dev_printk and __netdev_printk
o Remove now unused EXPORT_SYMBOL()s of __dev_printk and __netdev_printk
o Whitespace neatening
Signed-off-by: Joe Perches <joe@perches.com>
---
drivers/base/core.c | 57 ++++++++++++++++++++++++-----------------
include/linux/device.h | 11 +++-----
include/linux/netdevice.h | 3 --
lib/dynamic_debug.c | 61 ++++++++++++++++++++++++++++++++++++---------
net/core/dev.c | 5 ++-
5 files changed, 89 insertions(+), 48 deletions(-)
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 346be8b..320a32c 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -1844,25 +1844,19 @@ void device_shutdown(void)
*/
#ifdef CONFIG_PRINTK
-int __dev_printk(const char *level, const struct device *dev,
- struct va_format *vaf)
+int create_syslog_header(const struct device *dev, char *hdr, size_t hdrlen)
{
- char dict[128];
- size_t dictlen = 0;
const char *subsys;
-
- if (!dev)
- return printk("%s(NULL device *): %pV", level, vaf);
+ size_t pos = 0;
if (dev->class)
subsys = dev->class->name;
else if (dev->bus)
subsys = dev->bus->name;
else
- goto skip;
+ return 0;
- dictlen += snprintf(dict + dictlen, sizeof(dict) - dictlen,
- "SUBSYSTEM=%s", subsys);
+ pos += snprintf(hdr + pos, hdrlen - pos, "SUBSYSTEM=%s", subsys);
/*
* Add device identifier DEVICE=:
@@ -1878,28 +1872,41 @@ int __dev_printk(const char *level, const struct device *dev,
c = 'b';
else
c = 'c';
- dictlen++;
- dictlen += snprintf(dict + dictlen, sizeof(dict) - dictlen,
- "DEVICE=%c%u:%u",
- c, MAJOR(dev->devt), MINOR(dev->devt));
+ pos++;
+ pos += snprintf(hdr + pos, hdrlen - pos,
+ "DEVICE=%c%u:%u",
+ c, MAJOR(dev->devt), MINOR(dev->devt));
} else if (strcmp(subsys, "net") == 0) {
struct net_device *net = to_net_dev(dev);
- dictlen++;
- dictlen += snprintf(dict + dictlen, sizeof(dict) - dictlen,
- "DEVICE=n%u", net->ifindex);
+ pos++;
+ pos += snprintf(hdr + pos, hdrlen - pos,
+ "DEVICE=n%u", net->ifindex);
} else {
- dictlen++;
- dictlen += snprintf(dict + dictlen, sizeof(dict) - dictlen,
- "DEVICE=+%s:%s", subsys, dev_name(dev));
+ pos++;
+ pos += snprintf(hdr + pos, hdrlen - pos,
+ "DEVICE=+%s:%s", subsys, dev_name(dev));
}
-skip:
- return printk_emit(0, level[1] - '0',
- dictlen ? dict : NULL, dictlen,
+
+ return pos;
+}
+EXPORT_SYMBOL(create_syslog_header);
+
+static int __dev_printk(const char *level, const struct device *dev,
+ struct va_format *vaf)
+{
+ char hdr[128];
+ size_t hdrlen;
+
+ if (!dev)
+ return printk("%s(NULL device *): %pV", level, vaf);
+
+ hdrlen = create_syslog_header(dev, hdr, sizeof(hdr));
+
+ return printk_emit(0, level[1] - '0', hdrlen ? hdr : NULL, hdrlen,
"%s %s: %pV",
dev_driver_string(dev), dev_name(dev), vaf);
}
-EXPORT_SYMBOL(__dev_printk);
int dev_printk(const char *level, const struct device *dev,
const char *fmt, ...)
@@ -1914,6 +1921,7 @@ int dev_printk(const char *level, const struct device *dev,
vaf.va = &args;
r = __dev_printk(level, dev, &vaf);
+
va_end(args);
return r;
@@ -1933,6 +1941,7 @@ int func(const struct device *dev, const char *fmt, ...) \
vaf.va = &args; \
\
r = __dev_printk(kern_level, dev, &vaf); \
+ \
va_end(args); \
\
return r; \
diff --git a/include/linux/device.h b/include/linux/device.h
index 5083bcc..bd9701d 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -886,12 +886,12 @@ extern const char *dev_driver_string(const struct device *dev);
#ifdef CONFIG_PRINTK
-extern int __dev_printk(const char *level, const struct device *dev,
- struct va_format *vaf);
+extern int create_syslog_header(const struct device *dev,
+ char *hdr, size_t hdrlen);
+
extern __printf(3, 4)
int dev_printk(const char *level, const struct device *dev,
- const char *fmt, ...)
- ;
+ const char *fmt, ...);
extern __printf(2, 3)
int dev_emerg(const struct device *dev, const char *fmt, ...);
extern __printf(2, 3)
@@ -909,9 +909,6 @@ int _dev_info(const struct device *dev, const char *fmt, ...);
#else
-static inline int __dev_printk(const char *level, const struct device *dev,
- struct va_format *vaf)
-{ return 0; }
static inline __printf(3, 4)
int dev_printk(const char *level, const struct device *dev,
const char *fmt, ...)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index eb06e58..291e0ee 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2715,9 +2715,6 @@ static inline const char *netdev_name(const struct net_device *dev)
return dev->name;
}
-extern int __netdev_printk(const char *level, const struct net_device *dev,
- struct va_format *vaf);
-
extern __printf(3, 4)
int netdev_printk(const char *level, const struct net_device *dev,
const char *format, ...);
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 7ca29a0..6507d11 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -521,25 +521,23 @@ static char *dynamic_emit_prefix(const struct _ddebug *desc, char *buf)
int pos_after_tid;
int pos = 0;
- pos += snprintf(buf + pos, remaining(pos), "%s", KERN_DEBUG);
if (desc->flags & _DPRINTK_FLAGS_INCL_TID) {
if (in_interrupt())
- pos += snprintf(buf + pos, remaining(pos), "%s ",
- "<intr>");
+ pos += snprintf(buf + pos, remaining(pos), "<intr> ");
else
pos += snprintf(buf + pos, remaining(pos), "[%d] ",
- task_pid_vnr(current));
+ task_pid_vnr(current));
}
pos_after_tid = pos;
if (desc->flags & _DPRINTK_FLAGS_INCL_MODNAME)
pos += snprintf(buf + pos, remaining(pos), "%s:",
- desc->modname);
+ desc->modname);
if (desc->flags & _DPRINTK_FLAGS_INCL_FUNCNAME)
pos += snprintf(buf + pos, remaining(pos), "%s:",
- desc->function);
+ desc->function);
if (desc->flags & _DPRINTK_FLAGS_INCL_LINENO)
pos += snprintf(buf + pos, remaining(pos), "%d:",
- desc->lineno);
+ desc->lineno);
if (pos - pos_after_tid)
pos += snprintf(buf + pos, remaining(pos), " ");
if (pos >= PREFIX_SIZE)
@@ -559,9 +557,13 @@ int __dynamic_pr_debug(struct _ddebug *descriptor, const char *fmt, ...)
BUG_ON(!fmt);
va_start(args, fmt);
+
vaf.fmt = fmt;
vaf.va = &args;
- res = printk("%s%pV", dynamic_emit_prefix(descriptor, buf), &vaf);
+
+ res = printk(KERN_DEBUG "%s%pV",
+ dynamic_emit_prefix(descriptor, buf), &vaf);
+
va_end(args);
return res;
@@ -580,9 +582,24 @@ int __dynamic_dev_dbg(struct _ddebug *descriptor,
BUG_ON(!fmt);
va_start(args, fmt);
+
vaf.fmt = fmt;
vaf.va = &args;
- res = __dev_printk(dynamic_emit_prefix(descriptor, buf), dev, &vaf);
+
+ if (!dev) {
+ res = printk(KERN_DEBUG "(NULL device *): %pV", &vaf);
+ } else {
+ char dict[128];
+ size_t dictlen;
+
+ dictlen = create_syslog_header(dev, dict, sizeof(dict));
+
+ res = printk_emit(0, 7, dictlen ? dict : NULL, dictlen,
+ "%s%s %s: %pV",
+ dynamic_emit_prefix(descriptor, buf),
+ dev_driver_string(dev), dev_name(dev), &vaf);
+ }
+
va_end(args);
return res;
@@ -592,20 +609,40 @@ EXPORT_SYMBOL(__dynamic_dev_dbg);
#ifdef CONFIG_NET
int __dynamic_netdev_dbg(struct _ddebug *descriptor,
- const struct net_device *dev, const char *fmt, ...)
+ const struct net_device *dev, const char *fmt, ...)
{
struct va_format vaf;
va_list args;
int res;
- char buf[PREFIX_SIZE];
BUG_ON(!descriptor);
BUG_ON(!fmt);
va_start(args, fmt);
+
vaf.fmt = fmt;
vaf.va = &args;
- res = __netdev_printk(dynamic_emit_prefix(descriptor, buf), dev, &vaf);
+
+ if (dev && dev->dev.parent) {
+ char buf[PREFIX_SIZE];
+ char dict[128];
+ size_t dictlen;
+
+ dictlen = create_syslog_header(dev->dev.parent,
+ dict, sizeof(dict));
+
+ res = printk_emit(0, 7, dictlen ? dict : NULL, dictlen,
+ "%s%s %s: %pV",
+ dynamic_emit_prefix(descriptor, buf),
+ dev_driver_string(dev->dev.parent),
+ netdev_name(dev), &vaf);
+
+ } else if (dev) {
+ res = printk(KERN_DEBUG "%s: %pV", netdev_name(dev), &vaf);
+ } else {
+ res = printk(KERN_DEBUG "(NULL net_device): %pV", &vaf);
+ }
+
va_end(args);
return res;
diff --git a/net/core/dev.c b/net/core/dev.c
index 0ebaea1..f6ebe92 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6358,7 +6358,7 @@ const char *netdev_drivername(const struct net_device *dev)
return empty;
}
-int __netdev_printk(const char *level, const struct net_device *dev,
+static int __netdev_printk(const char *level, const struct net_device *dev,
struct va_format *vaf)
{
int r;
@@ -6373,7 +6373,6 @@ int __netdev_printk(const char *level, const struct net_device *dev,
return r;
}
-EXPORT_SYMBOL(__netdev_printk);
int netdev_printk(const char *level, const struct net_device *dev,
const char *format, ...)
@@ -6388,6 +6387,7 @@ int netdev_printk(const char *level, const struct net_device *dev,
vaf.va = &args;
r = __netdev_printk(level, dev, &vaf);
+
va_end(args);
return r;
@@ -6407,6 +6407,7 @@ int func(const struct net_device *dev, const char *fmt, ...) \
vaf.va = &args; \
\
r = __netdev_printk(level, dev, &vaf); \
+ \
va_end(args); \
\
return r; \
--
1.7.8.111.gad25c.dirty
^ permalink raw reply related
* Re: [PATCH] dynamic_debug: Restore dev_dbg/netdev_dbg functionality, reduce stack use
From: David Miller @ 2012-07-26 0:17 UTC (permalink / raw)
To: joe; +Cc: gregkh, jbaron, kay, jim.cromie, akpm, linux-kernel, netdev
In-Reply-To: <403af268697d3d117114b12000b20c74aafa5bac.1343261030.git.joe@perches.com>
From: Joe Perches <joe@perches.com>
Date: Wed, 25 Jul 2012 17:10:00 -0700
> commit c4e00daaa9 ("driver-core: extend dev_printk() to pass structured data")
> changed __dev_printk and broke dynamic-debug's ability to control the
> dynamic prefix of dev_dbg(dev,..).
>
> dynamic_emit_prefix() adds "[tid] module:func:line:" to the output and
> those additions got lost.
>
> In addition, the current dynamic debug code uses up to 3 recursion
> levels via %pV. This can consume quite a bit of stack. Directly
> call printk_emit to reduce the recursion by one depth.
>
> These changes include:
>
> o Remove KERN_DEBUG from dynamic_emit_prefix
> o Create and use function create_syslog_header to format the syslog
> header for printk_emit uses.
> o Call create_syslog_header and neaten __dev_printk
> o Call create_syslog_header and printk_emit from dynamic_dev_dbg
> o Call create_syslog_header and printk_emit from dynamic_netdev_dbg
> o Make __dev_printk and __netdev_printk static not global
> o Remove include header declarations of __dev_printk and __netdev_printk
> o Remove now unused EXPORT_SYMBOL()s of __dev_printk and __netdev_printk
> o Whitespace neatening
>
> Signed-off-by: Joe Perches <joe@perches.com>
For networking parts:
Acked-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* Re: [PATCH 00/16] Remove the ipv4 routing cache
From: David Miller @ 2012-07-26 0:54 UTC (permalink / raw)
To: alexander.duyck; +Cc: eric.dumazet, netdev
In-Reply-To: <20120725.163939.581743307449189972.davem@davemloft.net>
From: David Miller <davem@davemloft.net>
Date: Wed, 25 Jul 2012 16:39:39 -0700 (PDT)
> From: David Miller <davem@davemloft.net>
> Date: Wed, 25 Jul 2012 16:17:32 -0700 (PDT)
>
>> From: Alexander Duyck <alexander.duyck@gmail.com>
>> Date: Wed, 25 Jul 2012 16:02:45 -0700
>>
>>> Since your patches are in I have started to re-run my tests. I am
>>> seeing a significant drop in throughput with 8 flows which I expected,
>>> however it looks like one of the biggest issues I am seeing is that
>>> the dst_hold and dst_release calls seem to be causing some serious
>>> cache thrash. I was at 12.5Mpps w/ 8 flows before the patches, after
>>> your patches it drops to 8.3Mpps.
>>
>> Yes, this is something we knew would start happening.
>>
>> One idea is to make cached dsts be per-cpu in the nexthops.
>
> Actually I think what really kills your case is the removal of the
> noref path for route lookups. I'll work on a patch to restore that
> in the case where we use cached routes from the FIB nexthops.
Alex, here is something I tossed together, does it help with the
dst_hold()/dst_release() overhead at all?
diff --git a/include/net/route.h b/include/net/route.h
index c29ef27..8c52bc6 100644
--- a/include/net/route.h
+++ b/include/net/route.h
@@ -30,6 +30,7 @@
#include <net/inet_sock.h>
#include <linux/in_route.h>
#include <linux/rtnetlink.h>
+#include <linux/rcupdate.h>
#include <linux/route.h>
#include <linux/ip.h>
#include <linux/cache.h>
@@ -157,8 +158,22 @@ static inline struct rtable *ip_route_output_gre(struct net *net, struct flowi4
return ip_route_output_key(net, fl4);
}
-extern int ip_route_input(struct sk_buff *skb, __be32 dst, __be32 src,
- u8 tos, struct net_device *devin);
+extern int ip_route_input_noref(struct sk_buff *skb, __be32 dst, __be32 src,
+ u8 tos, struct net_device *devin);
+
+static inline int ip_route_input(struct sk_buff *skb, __be32 dst, __be32 src,
+ u8 tos, struct net_device *devin)
+{
+ int err;
+
+ rcu_read_lock();
+ err = ip_route_input_noref(skb, dst, src, tos, devin);
+ if (!err)
+ skb_dst_force(skb);
+ rcu_read_unlock();
+
+ return err;
+}
extern void ipv4_update_pmtu(struct sk_buff *skb, struct net *net, u32 mtu,
int oif, u32 mark, u8 protocol, int flow_flags);
diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index a0124eb..77e87af 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -827,7 +827,7 @@ static int arp_process(struct sk_buff *skb)
}
if (arp->ar_op == htons(ARPOP_REQUEST) &&
- ip_route_input(skb, tip, sip, 0, dev) == 0) {
+ ip_route_input_noref(skb, tip, sip, 0, dev) == 0) {
rt = skb_rtable(skb);
addr_type = rt->rt_type;
diff --git a/net/ipv4/ip_fragment.c b/net/ipv4/ip_fragment.c
index 7ad88e5..8d07c97 100644
--- a/net/ipv4/ip_fragment.c
+++ b/net/ipv4/ip_fragment.c
@@ -258,8 +258,8 @@ static void ip_expire(unsigned long arg)
/* skb dst is stale, drop it, and perform route lookup again */
skb_dst_drop(head);
iph = ip_hdr(head);
- err = ip_route_input(head, iph->daddr, iph->saddr,
- iph->tos, head->dev);
+ err = ip_route_input_noref(head, iph->daddr, iph->saddr,
+ iph->tos, head->dev);
if (err)
goto out_rcu_unlock;
diff --git a/net/ipv4/ip_input.c b/net/ipv4/ip_input.c
index 93134b0..bda8cac 100644
--- a/net/ipv4/ip_input.c
+++ b/net/ipv4/ip_input.c
@@ -339,8 +339,8 @@ static int ip_rcv_finish(struct sk_buff *skb)
* how the packet travels inside Linux networking.
*/
if (!skb_dst(skb)) {
- int err = ip_route_input(skb, iph->daddr, iph->saddr,
- iph->tos, skb->dev);
+ int err = ip_route_input_noref(skb, iph->daddr, iph->saddr,
+ iph->tos, skb->dev);
if (unlikely(err)) {
if (err == -EXDEV)
NET_INC_STATS_BH(dev_net(skb->dev),
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 3f7bb71..68887e3 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1199,10 +1199,9 @@ restart:
fnhe->fnhe_stamp = jiffies;
}
-static inline void rt_release_rcu(struct rcu_head *head)
+static inline void rt_free(struct rtable *rt)
{
- struct dst_entry *dst = container_of(head, struct dst_entry, rcu_head);
- dst_release(dst);
+ call_rcu_bh(&rt->dst.rcu_head, dst_rcu_free);
}
static void rt_cache_route(struct fib_nh *nh, struct rtable *rt)
@@ -1216,9 +1215,15 @@ static void rt_cache_route(struct fib_nh *nh, struct rtable *rt)
prev = cmpxchg(p, orig, rt);
if (prev == orig) {
- dst_clone(&rt->dst);
if (orig)
- call_rcu_bh(&orig->dst.rcu_head, rt_release_rcu);
+ rt_free(orig);
+ } else {
+ /* Routes we intend to cache in the FIB nexthop have
+ * the DST_NOCACHE bit set. However, if we are
+ * unsuccessful at storing this route into the cache
+ * we really need to clear that bit.
+ */
+ rt->dst.flags &= ~DST_NOCACHE;
}
}
@@ -1245,7 +1250,7 @@ static void rt_set_nexthop(struct rtable *rt, __be32 daddr,
#ifdef CONFIG_IP_ROUTE_CLASSID
rt->dst.tclassid = nh->nh_tclassid;
#endif
- if (!(rt->dst.flags & DST_HOST))
+ if (!(rt->dst.flags & DST_NOCACHE))
rt_cache_route(nh, rt);
}
@@ -1261,7 +1266,7 @@ static struct rtable *rt_dst_alloc(struct net_device *dev,
bool nopolicy, bool noxfrm, bool will_cache)
{
return dst_alloc(&ipv4_dst_ops, dev, 1, DST_OBSOLETE_FORCE_CHK,
- (will_cache ? 0 : DST_HOST) | DST_NOCACHE |
+ (will_cache ? 0 : (DST_HOST | DST_NOCACHE)) |
(nopolicy ? DST_NOPOLICY : 0) |
(noxfrm ? DST_NOXFRM : 0));
}
@@ -1588,8 +1593,9 @@ local_input:
if (!itag) {
rth = FIB_RES_NH(res).nh_rth_input;
if (rt_cache_valid(rth)) {
- dst_hold(&rth->dst);
- goto set_and_out;
+ skb_dst_set_noref(skb, &rth->dst);
+ err = 0;
+ goto out;
}
do_cache = true;
}
@@ -1620,7 +1626,6 @@ local_input:
}
if (do_cache)
rt_cache_route(&FIB_RES_NH(res), rth);
-set_and_out:
skb_dst_set(skb, &rth->dst);
err = 0;
goto out;
@@ -1658,8 +1663,8 @@ martian_source_keep_err:
goto out;
}
-int ip_route_input(struct sk_buff *skb, __be32 daddr, __be32 saddr,
- u8 tos, struct net_device *dev)
+int ip_route_input_noref(struct sk_buff *skb, __be32 daddr, __be32 saddr,
+ u8 tos, struct net_device *dev)
{
int res;
@@ -1702,7 +1707,7 @@ int ip_route_input(struct sk_buff *skb, __be32 daddr, __be32 saddr,
rcu_read_unlock();
return res;
}
-EXPORT_SYMBOL(ip_route_input);
+EXPORT_SYMBOL(ip_route_input_noref);
/* called with rcu_read_lock() */
static struct rtable *__mkroute_output(const struct fib_result *res,
diff --git a/net/ipv4/xfrm4_input.c b/net/ipv4/xfrm4_input.c
index 58d23a5..06814b6 100644
--- a/net/ipv4/xfrm4_input.c
+++ b/net/ipv4/xfrm4_input.c
@@ -27,8 +27,8 @@ static inline int xfrm4_rcv_encap_finish(struct sk_buff *skb)
if (skb_dst(skb) == NULL) {
const struct iphdr *iph = ip_hdr(skb);
- if (ip_route_input(skb, iph->daddr, iph->saddr,
- iph->tos, skb->dev))
+ if (ip_route_input_noref(skb, iph->daddr, iph->saddr,
+ iph->tos, skb->dev))
goto drop;
}
return dst_input(skb);
^ permalink raw reply related
* Re: [PATCH] sctp: Make "Invalid Stream Identifier" ERROR follows SACK when bundling
From: Xufeng Zhang @ 2012-07-26 1:30 UTC (permalink / raw)
To: Vlad Yasevich
Cc: Neil Horman, xufeng zhang, sri, davem, linux-sctp, netdev,
linux-kernel
In-Reply-To: <50100A29.8010705@gmail.com>
On 7/25/12, Vlad Yasevich <vyasevich@gmail.com> wrote:
>> And after take a moment to look into the relative codes, I think we
>> can implement it
>> by below way:
>> 1). Add a flag(isi_err_needed) in the embedded struct peer of struct
>> struct sctp_association
>> just like sack_needed flag.
>> 2). When "invalid stream identifier" ERROR happens in sctp_eat_data()
>> function, we just
>> set isi_err_needed flag and don't create ERROR chunk and also don't
>> insert SCTP_CMD_REPLY command.
>> 3). In sctp_gen_sack() function, we create ERROR chunk and also insert
>> SCTP_CMD_REPLY command if isi_err_needed flag is set.
>>
>> Is this way proper?
>>
>
> So, I looked at the code, and it looks very simple to do. We already
> return a specific status from sctp_eat_data() when the error was
> generated. All you have to do is take the code that generates the error
> and adds it to the command list and give it its own small function that
> you can then call if SCTP_IERROR_BAD_STREAM error was returned.
No, it will still has the same problem by just doing this.
SCTP_CMD_GEN_SACK command actually don't enqueue SACK to outqueue,
sctp_gen_sack() do this things when processing SCTP_CMD_GEN_SACK command
in sctp_cmd_interpreter().
So it's not enough if we just insert SCTP_ERROR_INV_STRM command after
sctp_eat_data() return SCTP_IERROR_BAD_STREAM in sctp_sf_eat_data_6_2().
Thanks,
Xufeng Zhang
>
> -vlad
>
>>
>> Thanks,
>> Xufeng Zhang
>>>
>>>
>>>
>>> Thanks,
>>> Xufeng Zhang
>>>>
>>>> -vlad
>>>>>
>>>>> Thanks,
>>>>> Xufeng Zhang
>>>>>>
>>>>>> -vlad
>>>>
>>>>
>>>> --
>>>> Sent from my Android phone with SkitMail. Please excuse my brevity.
>>>>
>>>
>
>
>
^ permalink raw reply
* Re: [PATCH] sctp: Make "Invalid Stream Identifier" ERROR follows SACK when bundling
From: Xufeng Zhang @ 2012-07-26 1:34 UTC (permalink / raw)
To: Neil Horman
Cc: Vlad Yasevich, xufeng zhang, sri, davem, linux-sctp, netdev,
linux-kernel
In-Reply-To: <20120725112701.GB4366@hmsreliant.think-freely.org>
On 7/25/12, Neil Horman <nhorman@tuxdriver.com> wrote:
> On Wed, Jul 25, 2012 at 05:22:19PM +0800, Xufeng Zhang wrote:
>> On 7/25/12, Xufeng Zhang <xufengzhang.main@gmail.com> wrote:
>> > On 7/25/12, Vlad Yasevich <vyasevich@gmail.com> wrote:
>> >>
>> >> Actually not true. AUTH can be before SACK. So can any addip chunks
>> >> that
>> >> aid in locating an association.
>> >>
>> >> Now AUTH isn't a big issue since its autogenerated to the packet but
>> >> ADDIP
>> >> is since it could be queued up for retransmission.
>> >>
>> >> There could be other extensions as well. It really needs to be done
>> >> either
>> >> through side effects or making error chunks go at the end of other
>> >> control
>> >> chunks. Need to audit the spec to see if that's ok.
>> > You are right, I just found SHUTDOWN chunks are also before SACK based
>> > on
>> > your commit "[SCTP]: Fix SACK sequence during shutdown".
>> > Maybe the only solution is to do some work on side effects just as you
>> > said.
>> > Thanks for your explanation!
>>
>> And after take a moment to look into the relative codes, I think we
>> can implement it
>> by below way:
>> 1). Add a flag(isi_err_needed) in the embedded struct peer of struct
>> struct sctp_association
>> just like sack_needed flag.
>> 2). When "invalid stream identifier" ERROR happens in sctp_eat_data()
>> function, we just
>> set isi_err_needed flag and don't create ERROR chunk and also don't
>> insert SCTP_CMD_REPLY command.
>> 3). In sctp_gen_sack() function, we create ERROR chunk and also insert
>> SCTP_CMD_REPLY command if isi_err_needed flag is set.
>>
>> Is this way proper?
>>
> That would probably work yes. Another way might just be to do some
> re-ordering
> in sctp_outq_flush. Before processing the control chunk list, scan it,
> and:
> 1) move all error chunks to the head of the list
> 2) move all sack chunks to the head of the list
> 3) move all shutdown chunks to the head of the list
>
> You can do that in a single iteration of the list if you use a few on-stack
> lists and list_splice
Thank you very much for your suggestion!
I'll compare it with side effects modification and make a decision.
Thanks,
Xufeng Zhang
>
> Neil
>
>>
>> Thanks,
>> Xufeng Zhang
>> >
>> >
>> >
>> > Thanks,
>> > Xufeng Zhang
>> >>
>> >> -vlad
>> >>>
>> >>>Thanks,
>> >>>Xufeng Zhang
>> >>>>
>> >>>> -vlad
>> >>
>> >>
>> >> --
>> >> Sent from my Android phone with SkitMail. Please excuse my brevity.
>> >>
>> >
>>
>
^ permalink raw reply
* Dear Email Owner(You Won Promo) !!!
From: Royal Dutch Shell® @ 2012-07-26 2:11 UTC (permalink / raw)
Your Email address Won a Sum payout of $2,000 000.00 USD. As our Royal
Dutch Shell Grant, in cash credited to You, Verify this mail by providing
your Complete Details, to our Claims Agent more so, Please endeavor to quote
your Qualification numbers (NL-222-6747,NG-900-56) in all dealings.
====================
Contact Person: Mr. Taylor Keith.
Email: claimsagentkeith@zing.vn
====================
1.FULL NAMES:_________
2.ADDRESS:_________
3.COUNTRY___________
4.AGE___________
5.SEX:________
6.OCCUPATION:________
7.TELEPHONE:_______
8.Next of Kin___________
All email addresses entered for this promotional draws were randomly
inputted from an internet resource database using the Synchronized Random
Selection System (SRSS)keep all your winning details confidential
until you claim your prize.
Best Regards,
Miss.Dorothy White
********************************************************************
This email and any files transmitted with it are confidential and
intended solely for the use of the individual or entity to whom they
are addressed.
*********************************************************************
^ permalink raw reply
* Re: [PATCH 00/16] Remove the ipv4 routing cache
From: Alexander Duyck @ 2012-07-26 2:30 UTC (permalink / raw)
To: David Miller; +Cc: eric.dumazet, netdev
In-Reply-To: <20120725.175406.1331203183232530233.davem@davemloft.net>
On Wed, Jul 25, 2012 at 5:54 PM, David Miller <davem@davemloft.net> wrote:
> From: David Miller <davem@davemloft.net>
> Date: Wed, 25 Jul 2012 16:39:39 -0700 (PDT)
>
>> From: David Miller <davem@davemloft.net>
>> Date: Wed, 25 Jul 2012 16:17:32 -0700 (PDT)
>>
>>> From: Alexander Duyck <alexander.duyck@gmail.com>
>>> Date: Wed, 25 Jul 2012 16:02:45 -0700
>>>
>>>> Since your patches are in I have started to re-run my tests. I am
>>>> seeing a significant drop in throughput with 8 flows which I expected,
>>>> however it looks like one of the biggest issues I am seeing is that
>>>> the dst_hold and dst_release calls seem to be causing some serious
>>>> cache thrash. I was at 12.5Mpps w/ 8 flows before the patches, after
>>>> your patches it drops to 8.3Mpps.
>>>
>>> Yes, this is something we knew would start happening.
>>>
>>> One idea is to make cached dsts be per-cpu in the nexthops.
>>
>> Actually I think what really kills your case is the removal of the
>> noref path for route lookups. I'll work on a patch to restore that
>> in the case where we use cached routes from the FIB nexthops.
>
> Alex, here is something I tossed together, does it help with the
> dst_hold()/dst_release() overhead at all?
>
I downloaded your patch, applied it, rebuilt the kernel, and then
rebooted. However after the reboot I am unable to access the system
remotely. It looks like I will have to wait until I get into the
office tomorrow to figure out what happened to the system.
I will get you an update on if it helped or not once I figure out what
happened in the morning.
Thanks,
Alex
^ permalink raw reply
* Re: [PATCH] sctp: Make "Invalid Stream Identifier" ERROR follows SACK when bundling
From: Vlad Yasevich @ 2012-07-26 2:45 UTC (permalink / raw)
To: Xufeng Zhang
Cc: Neil Horman, xufeng zhang, sri, davem, linux-sctp, netdev,
linux-kernel
In-Reply-To: <CA+=dFzj11c9K0TS0T7D1aBVz+ppdFK91ZzBbevLV23FrWJNzmw@mail.gmail.com>
Xufeng Zhang <xufengzhang.main@gmail.com> wrote:
>On 7/25/12, Vlad Yasevich <vyasevich@gmail.com> wrote:
>>> And after take a moment to look into the relative codes, I think we
>>> can implement it
>>> by below way:
>>> 1). Add a flag(isi_err_needed) in the embedded struct peer of struct
>>> struct sctp_association
>>> just like sack_needed flag.
>>> 2). When "invalid stream identifier" ERROR happens in
>sctp_eat_data()
>>> function, we just
>>> set isi_err_needed flag and don't create ERROR chunk and also don't
>>> insert SCTP_CMD_REPLY command.
>>> 3). In sctp_gen_sack() function, we create ERROR chunk and also
>insert
>>> SCTP_CMD_REPLY command if isi_err_needed flag is set.
>>>
>>> Is this way proper?
>>>
>>
>> So, I looked at the code, and it looks very simple to do. We already
>> return a specific status from sctp_eat_data() when the error was
>> generated. All you have to do is take the code that generates the
>error
>> and adds it to the command list and give it its own small function
>that
>> you can then call if SCTP_IERROR_BAD_STREAM error was returned.
>
>No, it will still has the same problem by just doing this.
>SCTP_CMD_GEN_SACK command actually don't enqueue SACK to outqueue,
>sctp_gen_sack() do this things when processing SCTP_CMD_GEN_SACK
>command
>in sctp_cmd_interpreter().
>So it's not enough if we just insert SCTP_ERROR_INV_STRM command after
>sctp_eat_data() return SCTP_IERROR_BAD_STREAM in
>sctp_sf_eat_data_6_2().
>
>
All you have to do is change the order of side effect commands and the above is a guide. Ill prototype it tomorrow when I have time.
-vlad
>
>Thanks,
>Xufeng Zhang
>
>>
>> -vlad
>>
>>>
>>> Thanks,
>>> Xufeng Zhang
>>>>
>>>>
>>>>
>>>> Thanks,
>>>> Xufeng Zhang
>>>>>
>>>>> -vlad
>>>>>>
>>>>>> Thanks,
>>>>>> Xufeng Zhang
>>>>>>>
>>>>>>> -vlad
>>>>>
>>>>>
>>>>> --
>>>>> Sent from my Android phone with SkitMail. Please excuse my
>brevity.
>>>>>
>>>>
>>
>>
>>
--
Sent from my Android phone with SkitMail. Please excuse my brevity.
^ permalink raw reply
* Re: [PATCH] sctp: Make "Invalid Stream Identifier" ERROR follows SACK when bundling
From: Xufeng Zhang @ 2012-07-26 2:50 UTC (permalink / raw)
To: Vlad Yasevich
Cc: Neil Horman, xufeng zhang, sri, davem, linux-sctp, netdev,
linux-kernel
In-Reply-To: <CA+=dFzj11c9K0TS0T7D1aBVz+ppdFK91ZzBbevLV23FrWJNzmw@mail.gmail.com>
On 7/26/12, Xufeng Zhang <xufengzhang.main@gmail.com> wrote:
> On 7/25/12, Vlad Yasevich <vyasevich@gmail.com> wrote:
>>> And after take a moment to look into the relative codes, I think we
>>> can implement it
>>> by below way:
>>> 1). Add a flag(isi_err_needed) in the embedded struct peer of struct
>>> struct sctp_association
>>> just like sack_needed flag.
>>> 2). When "invalid stream identifier" ERROR happens in sctp_eat_data()
>>> function, we just
>>> set isi_err_needed flag and don't create ERROR chunk and also don't
>>> insert SCTP_CMD_REPLY command.
>>> 3). In sctp_gen_sack() function, we create ERROR chunk and also insert
>>> SCTP_CMD_REPLY command if isi_err_needed flag is set.
>>>
>>> Is this way proper?
>>>
>>
>> So, I looked at the code, and it looks very simple to do. We already
>> return a specific status from sctp_eat_data() when the error was
>> generated. All you have to do is take the code that generates the error
>> and adds it to the command list and give it its own small function that
>> you can then call if SCTP_IERROR_BAD_STREAM error was returned.
>
> No, it will still has the same problem by just doing this.
> SCTP_CMD_GEN_SACK command actually don't enqueue SACK to outqueue,
> sctp_gen_sack() do this things when processing SCTP_CMD_GEN_SACK command
> in sctp_cmd_interpreter().
> So it's not enough if we just insert SCTP_ERROR_INV_STRM command after
> sctp_eat_data() return SCTP_IERROR_BAD_STREAM in sctp_sf_eat_data_6_2().
Yes, I just tried this way, SACK is still bundled after ERROR chunk.
But I think my above method is also not fine if there are multiple
error DATA chunks
bundled in a packet.
Really awesome!
Thanks,
Xufeng Zhang
>
>
>
> Thanks,
> Xufeng Zhang
>
>>
>> -vlad
>>
>>>
>>> Thanks,
>>> Xufeng Zhang
>>>>
>>>>
>>>>
>>>> Thanks,
>>>> Xufeng Zhang
>>>>>
>>>>> -vlad
>>>>>>
>>>>>> Thanks,
>>>>>> Xufeng Zhang
>>>>>>>
>>>>>>> -vlad
>>>>>
>>>>>
>>>>> --
>>>>> Sent from my Android phone with SkitMail. Please excuse my brevity.
>>>>>
>>>>
>>
>>
>>
>
^ permalink raw reply
* Re: [PATCH] sctp: Make "Invalid Stream Identifier" ERROR follows SACK when bundling
From: Vlad Yasevich @ 2012-07-26 2:55 UTC (permalink / raw)
To: Xufeng Zhang
Cc: Neil Horman, xufeng zhang, sri, davem, linux-sctp, netdev,
linux-kernel
In-Reply-To: <CA+=dFziK09TVcK29x1SLg+JanOj6o__bJKf8yohw6dTYCUCF5A@mail.gmail.com>
Xufeng Zhang <xufengzhang.main@gmail.com> wrote:
>On 7/26/12, Xufeng Zhang <xufengzhang.main@gmail.com> wrote:
>> On 7/25/12, Vlad Yasevich <vyasevich@gmail.com> wrote:
>>>> And after take a moment to look into the relative codes, I think we
>>>> can implement it
>>>> by below way:
>>>> 1). Add a flag(isi_err_needed) in the embedded struct peer of
>struct
>>>> struct sctp_association
>>>> just like sack_needed flag.
>>>> 2). When "invalid stream identifier" ERROR happens in
>sctp_eat_data()
>>>> function, we just
>>>> set isi_err_needed flag and don't create ERROR chunk and also don't
>>>> insert SCTP_CMD_REPLY command.
>>>> 3). In sctp_gen_sack() function, we create ERROR chunk and also
>insert
>>>> SCTP_CMD_REPLY command if isi_err_needed flag is set.
>>>>
>>>> Is this way proper?
>>>>
>>>
>>> So, I looked at the code, and it looks very simple to do. We
>already
>>> return a specific status from sctp_eat_data() when the error was
>>> generated. All you have to do is take the code that generates the
>error
>>> and adds it to the command list and give it its own small function
>that
>>> you can then call if SCTP_IERROR_BAD_STREAM error was returned.
>>
>> No, it will still has the same problem by just doing this.
>> SCTP_CMD_GEN_SACK command actually don't enqueue SACK to outqueue,
>> sctp_gen_sack() do this things when processing SCTP_CMD_GEN_SACK
>command
>> in sctp_cmd_interpreter().
>> So it's not enough if we just insert SCTP_ERROR_INV_STRM command
>after
>> sctp_eat_data() return SCTP_IERROR_BAD_STREAM in
>sctp_sf_eat_data_6_2().
>
>Yes, I just tried this way, SACK is still bundled after ERROR chunk.
>But I think my above method is also not fine if there are multiple
>error DATA chunks
>bundled in a packet.
>Really awesome!
1. Catch the error return.
2. Set flag indicating error is needed.
3. Queue sack as needed.
4. If error flag set call new function to queue error chunk.
That should fix things. Do this in all callers of sctp_eat_data.
-vlad
>
>
>Thanks,
>Xufeng Zhang
>
>>
>>
>>
>> Thanks,
>> Xufeng Zhang
>>
>>>
>>> -vlad
>>>
>>>>
>>>> Thanks,
>>>> Xufeng Zhang
>>>>>
>>>>>
>>>>>
>>>>> Thanks,
>>>>> Xufeng Zhang
>>>>>>
>>>>>> -vlad
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Xufeng Zhang
>>>>>>>>
>>>>>>>> -vlad
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Sent from my Android phone with SkitMail. Please excuse my
>brevity.
>>>>>>
>>>>>
>>>
>>>
>>>
>>
--
Sent from my Android phone with SkitMail. Please excuse my brevity.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox