* Re: Throughput regression with `tcp: refine TSO autosizing`
From: Michal Kazior @ 2015-02-03 8:44 UTC (permalink / raw)
To: Eric Dumazet; +Cc: linux-wireless, Network Development, eyalpe
In-Reply-To: <1422903136.21689.114.camel@edumazet-glaptop2.roam.corp.google.com>
On 2 February 2015 at 19:52, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Mon, 2015-02-02 at 11:27 +0100, Michal Kazior wrote:
>
>> While testing I've had my internal GRO patch for ath10k and no stretch
>> ack patches.
>
> Thanks for the data, I took a look at it.
>
> I am afraid this GRO patch might be the problem.
The entire performance drop happens without the GRO patch as well. I
tested with it included because I intended to upstream it later. I'll
run without it in future tests.
[...]
> Could you make again your experiments using upstream kernel (David
> Miller net tree) ?
Sure.
> You also could post the GRO patch so that we can comment on it.
(You probably want to see mac80211 patch as well:
06d181a8fd58031db9c114d920b40d8820380a6e "mac80211: add NAPI support
back")
diff --git a/drivers/net/wireless/ath/ath10k/core.c
b/drivers/net/wireless/ath/ath10k/core.c
index 36a8fcf..367e896 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -1147,6 +1147,12 @@ err:
}
EXPORT_SYMBOL(ath10k_core_start);
+static int ath10k_core_napi_dummy_poll(struct napi_struct *napi, int budget)
+{
+ WARN_ON(1);
+ return 0;
+}
+
int ath10k_wait_for_suspend(struct ath10k *ar, u32 suspend_opt)
{
int ret;
@@ -1414,6 +1420,10 @@ struct ath10k *ath10k_core_create(size_t
priv_size, struct device *dev,
INIT_WORK(&ar->register_work, ath10k_core_register_work);
INIT_WORK(&ar->restart_work, ath10k_core_restart);
+ init_dummy_netdev(&ar->napi_dev);
+ ieee80211_napi_add(ar->hw, &ar->napi, &ar->napi_dev,
+ ath10k_core_napi_dummy_poll, 64);
+
ret = ath10k_debug_create(ar);
if (ret)
goto err_free_wq;
@@ -1434,6 +1444,7 @@ void ath10k_core_destroy(struct ath10k *ar)
{
flush_workqueue(ar->workqueue);
destroy_workqueue(ar->workqueue);
+ netif_napi_del(&ar->napi);
ath10k_debug_destroy(ar);
ath10k_mac_destroy(ar);
diff --git a/drivers/net/wireless/ath/ath10k/core.h
b/drivers/net/wireless/ath/ath10k/core.h
index 2d9f871..b5a8847 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -623,6 +623,9 @@ struct ath10k {
struct dfs_pattern_detector *dfs_detector;
+ struct net_device napi_dev;
+ struct napi_struct napi;
+
#ifdef CONFIG_ATH10K_DEBUGFS
struct ath10k_debug debug;
#endif
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c
b/drivers/net/wireless/ath/ath10k/htt_rx.c
index c1da44f..7e58b38 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -2061,5 +2061,7 @@ static void ath10k_htt_txrx_compl_task(unsigned long ptr)
ath10k_htt_rx_in_ord_ind(ar, skb);
dev_kfree_skb_any(skb);
}
+
+ napi_gro_flush(&htt->ar->napi, false);
spin_unlock_bh(&htt->rx_ring.lock);
}
So that you can quickly get an understanding how ath10k Rx works:
first tasklet (not visible in the patch) picks up smallish event
buffers from firmware and puts them into ath10k queue for latter
processing by another tasklet (the last hunk). Each such event buffer
is just some metainfo but can "carry" tens of frames (both Rx and Tx
completions). The count is arbitrary and depends on fw/hw combo and
air conditions. The GRO flush is called after all queued small event
buffers are processed (frames delivered up to mac80211 which can in
turn perform aggregation reordering in case some frames were
re-transmitted in the meantime before handing them to net subsystem).
Michał
^ permalink raw reply related
* Re: [PATCHv2 net] net: restore lro after device detached from bridge
From: Alexander Duyck @ 2015-02-03 8:37 UTC (permalink / raw)
To: Fan Du; +Cc: Fan Du, bhutchings, davem, netdev
In-Reply-To: <54D0740A.6050701@gmail.com>
On 02/03/2015 08:08 AM, Fan Du wrote:
> 于 2015年02月02日 18:35, Alexander Duyck 写道:
>> On 02/01/2015 06:20 PM, Fan Du wrote:
>>> 于 2015年01月31日 04:48, Alexander Duyck 写道:
>>>> On 01/30/2015 04:33 AM, Fan Du wrote:
>>>>> Either detaching a device from bridge or switching a device
>>>>> out of FORWARDING state, the original lro feature should
>>>>> possibly be enabled for good reason, e.g. hw feature like
>>>>> receive side coalescing could come into play.
>>>>>
>>>>> BEFORE:
>>>>> echo 1 > /proc/sys/net/ipv4/conf/ens806f0/forwarding && ethtool -k
>>>>> ens806f0 | grep large
>>>>> large-receive-offload: off
>>>>>
>>>>> echo 0 > /proc/sys/net/ipv4/conf/ens806f0/forwarding && ethtool -k
>>>>> ens806f0 | grep large
>>>>> large-receive-offload: off
>>>>>
>>>>> AFTER:
>>>>> echo 1 > /proc/sys/net/ipv4/conf/ens806f0/forwarding && ethtool -k
>>>>> ens806f0 | grep large
>>>>> large-receive-offload: off
>>>>>
>>>>> echo 0 > /proc/sys/net/ipv4/conf/ens806f0/forwarding && ethtool -k
>>>>> ens806f0 | grep large
>>>>> large-receive-offload: on
>>>>>
>>>>> Signed-off-by: Fan Du <fan.du@intel.com>
>>>>> Fixes: 0187bdfb0567 ("net: Disable LRO on devices that are
>>>>> forwarding")
>>>>
>>>
>>>> First off this isn't a "fix". This is going to likely break more than
>>>> it fixes. The main reason why LRO is disabled is because it can cause
>>>> more harm then it helps. Since GRO is available we should err on the
>>>> side of caution since enabling LRO/RSC can have undesirable side
>>>> effects
>>>> in a number of cases.
>>>
>>> I think you are talking about bad scenarios when net device is
>>> attached to a bridge.
>>> Then what's the good reason user has to pay extra cpu power for
>>> using GRO, instead
>>> of using hw capable LRO/RSC when this net device is detached from
>>> bridge acting as
>>> a standalone NIC?
>>>
>>> Note, SRC is defaulted to *ON* in practice for ALL ixgbe NICs, as
>>> same other RSC capable
>>> NICs. Attaching net device to a bridge _once_ should not changed its
>>> default configuration,
>>> moreover it's a subtle change without any message that user won't
>>> noticed at all.
>
>> No, RSC only has benefits for IPv4/TCP large packets. However
>> historically there have been issues seen w/ small packet performance
>> with RSC enabled.
>
> Only when parallel client exceeds 4, gro trumps lro performance on my
> testbed for small packets.
> The difference comes from the fact that TCP RSS hash flows from
> clients into different NIC queues
> for multiple cpu, while RSC engine inside NIC has limit resource
> compared with that of cpu used by gro.
>
> NICs: 82599EB
> server:ipserf -s -B 192.168.5.1
> client:iperf -c 192.168.5.1 -i 1 -M 100 -P x
>
> -P Bandwidth/lro on Bandwidth/lro off
> gro off gro on
>
> 1 2.31 Gbits/sec 947 Mbits/sec
> 2 3.09 Gbits/sec 1.97 Gbits/sec
> 3 3.19 Gbits/sec 2.70 Gbits/sec
> 4 3.16 Gbits/sec 3.39 Gbits/sec
> 5 3.23 Gbits/sec 3.33 Gbits/sec
> 6 3.19 Gbits/sec 3.74 Gbits/sec
> 7 3.18 Gbits/sec 3.88 Gbits/sec
> 8 3.17 Gbits/sec 3.24 Gbits/sec
> 9 3.16 Gbits/sec 3.70 Gbits/sec
> 10 3.15 Gbits/sec 3.76 Gbits/sec
> 11 3.10 Gbits/sec 4.03 Gbits/sec
> 12 3.11 Gbits/sec 3.13 Gbits/sec
> 13 3.12 Gbits/sec 4.12 Gbits/sec
> 14 3.07 Gbits/sec 4.04 Gbits/sec
> 15 3.03 Gbits/sec 3.14 Gbits/sec
> 16 2.99 Gbits/sec 3.93 Gbits/sec
>
>
>
>
> Some have been addressed, however there are still
The point I think you are not getting is that bulk throughput
performance does not justify enabling a feature that may impact
stability or possibly harm small packet performance.
There are more reasons than routing and bridging to disable LRO. Those
two reasons though were so bad that we couldn't allow end users to
possibly encounter them so we disabled the feature for them.
There are a number of other cases where LRO might be disabled as in the
possible latency case I reported. As such you should not be enabling
LRO just because only two of the possible issues have now been addressed.
It is best to leave this up to the end-user to re-enable. If you are
seeing the feature disabled as a result of some init script on the
system you may want to look at re-enabling it as a part of some other
init script that you use when disabling routing or bridging.
- Alex
^ permalink raw reply
* pull request: bluetooth-next 2015-02-03
From: Johan Hedberg @ 2015-02-03 8:21 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-bluetooth
[-- Attachment #1: Type: text/plain, Size: 5731 bytes --]
Hi Dave,
Here's what's likely the last bluetooth-next pull request for 3.20.
Notable changes include:
- xHCI workaround + a new id for the ath3k driver
- Several new ids for the btusb driver
- Support for new Intel Bluetooth controllers
- Minor cleanups to ieee802154 code
- Nested sleep warning fix in socket accept() code path
- Fixes for Out of Band pairing handling
- Support for LE scan restarting for HCI_QUIRK_STRICT_DUPLICATE_FILTER
- Improvements to data we expose through debugfs
- Proper handling of Hardware Error HCI events
Please let me know if there are any issues pulling. Thanks.
Johan
---
The following changes since commit 0c49087462e8587c12ecfeaf1dd46fdc0ddc4532:
Merge tag 'mac80211-next-for-davem-2015-01-19' of git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next (2015-01-19 16:22:19 -0500)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git for-upstream
for you to fetch changes up to 88d9077c27d0c1a7c022d9dc987640beecf23560:
Bluetooth: Fix potential NULL dereference (2015-02-03 09:02:12 +0100)
----------------------------------------------------------------
Adam Lee (1):
Bluetooth: ath3k: workaround the compatibility issue with xHCI controller
Dmitry Tunin (1):
Bluetooth: ath3k: Add support of AR3012 bluetooth 13d3:3423 device
Jakub Pawlowski (4):
Bluetooth: Set HCI_QUIRK_STRICT_DUPLICATE_FILTER for BTUSB_ATH3012
Bluetooth: Set HCI_QUIRK_STRICT_DUPLICATE_FILTER for BTUSB_INTEL
Bluetooth: Add le_scan_restart work for LE scan restarting
Bluetooth: Add restarting to service discovery
Johan Hedberg (12):
Bluetooth: Check for valid bdaddr in add_remote_oob_data
Bluetooth: Remove incorrect check for BDADDR_BREDR address type
Bluetooth: Convert Set SC to use HCI Request
Bluetooth: Enforce zero-valued hash/rand192 for LE OOB
Bluetooth: btusb: Remove redundant call to btusb_free_frags()
Bluetooth: Fix check for SSP when enabling SC
Bluetooth: Fix notifying discovery state upon reset
Bluetooth: Fix notifying discovery state when powering off
Bluetooth: btusb: Fix race when waiting for BTUSB_DOWNLOADING
Bluetooth: btusb: Use wait_on_bit_timeout() for BTUSB_BOOTING
Bluetooth: Remove mgmt_rp_read_local_oob_ext_data struct
Bluetooth: Fix potential NULL dereference
Marcel Holtmann (28):
Bluetooth: Fix dependency for BR/EDR Secure Connections mode on SSP
Bluetooth: Limit BR/EDR switching for LE only with secure connections
Bluetooth: Require SSP enabling before BR/EDR Secure Connections
Bluetooth: btusb: Add support for Dynex/Insignia USB dongles
Bluetooth: btusb: Add firmware loading for Intel Snowfield Peak devices
Bluetooth: Clear P-192 values for OOB when in Secure Connections Only mode
Bluetooth: Use helper function to determine BR/EDR OOB data present
Bluetooth: Check for P-256 OOB values in Secure Connections Only mode
Bluetooth: btusb: Handle out of order firmware loading complete event
Bluetooth: Introduce hci_dev_do_reset helper function
Bluetooth: Perform a power cycle when receiving hardware error event
Bluetooth: btusb: Provide hardware error handler for Intel devices
Bluetooth: Move smp_unregister() into hci_dev_do_close() function
Bluetooth: btusb: Sort USB_DEVICE entries for Marvell by vendor id
Bluetooth: btusb: Ignore unknown Intel devices with generic descriptor
Bluetooth: btusb: Add support for USB based AMP controllers
Bluetooth: btusb: Limit hardware error handling to Intel Snowfield Peak
Bluetooth: Store OOB data present value for each set of remote OOB data
Bluetooth: Fix OOB data present value for BR/EDR Secure Connections
Bluetooth: Fix OOB data present value for SMP pairing
Bluetooth: Allow remote OOB data to only provide P-192 or P-256 values
Bluetooth: Expose Secure Simple Pairing debug mode setting in debugfs
Bluetooth: Track changes from HCI Write Simple Pairing Debug Mode command
Bluetooth: Expose debug keys usage setting via debugfs
Bluetooth: Expose hardware error code as debugfs entry
Bluetooth: Expose remote OOB information as debugfs entry
Bluetooth: Fix OOB data present for BR/EDR Secure Connections Only mode
Bluetooth: Set HCI_QUIRK_STRICT_DUPLICATE_FILTER for BTUSB_INTEL_NEW
Matej Dubovy (1):
Bluetooth: btusb: Add support for Lite-On (04ca) Broadcom based, BCM43142
Mohammad Jamal (2):
ieee802154: cc2520: Replace shift operations by BIT macro
ieee802154: cc2520: Fix space before , coding style issue
Peter Hurley (1):
Bluetooth: Fix nested sleeps
Rick Dunn (1):
Bluetooth: btusb: Add Broadcom patchram support for ASUSTek devices
Szymon Janc (2):
Bluetooth: Fix reporting invalid RSSI for LE devices
Bluetooth: Fix sending Read Remote Extended Features command
drivers/bluetooth/ath3k.c | 10 +
drivers/bluetooth/btusb.c | 671 +++++++++++++++++++++++++++++++++++++-
drivers/net/ieee802154/cc2520.c | 10 +-
include/net/bluetooth/hci_core.h | 11 +-
include/net/bluetooth/mgmt.h | 4 -
net/bluetooth/bnep/core.c | 7 +-
net/bluetooth/hci_core.c | 169 ++++++++--
net/bluetooth/hci_debugfs.c | 79 ++++-
net/bluetooth/hci_event.c | 135 ++++++--
net/bluetooth/l2cap_sock.c | 9 +-
net/bluetooth/mgmt.c | 279 +++++++++++-----
net/bluetooth/rfcomm/sock.c | 9 +-
net/bluetooth/sco.c | 8 +-
net/bluetooth/smp.c | 2 +-
14 files changed, 1214 insertions(+), 189 deletions(-)
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* RE: [PATCH] net: rocker: Change netdev names to include slot number
From: Rosen, Rami @ 2015-02-03 7:33 UTC (permalink / raw)
To: David Ahern, Florian Fainelli, Scott Feldman; +Cc: Netdev
In-Reply-To: <54CFF5B3.7070509@gmail.com>
Hi,
>root@f21 ~]# cat /sys/devices/virtual/net/sw5p0/phys_port_id
>cat: /sys/devices/virtual/net/sw5p0/phys_port_id: Operation not supported
Just a side note: commit aecbe01e7410 "net-sysfs: expose physical switch id for particular device" added support for this entry, relying on the fact that the ndo_switch_parent_id_get() is implemented (which seems to be a valid assumption for switch devices) and that the dev_isalive() condition is met.
Could it be that the reason for getting -EOPNOTSUPP was that the ndo_switch_parent_id_get() is not implemented in your switch device or that the condition was not met in your test?
Rami Rosen
Intel Corporation
^ permalink raw reply
* [PATCH v3] gianfar: correct the bad expression while writing bit-pattern
From: Sanjeev Sharma @ 2015-02-03 7:32 UTC (permalink / raw)
To: claudiu.manoil
Cc: davem, matei.pavaluca, netdev, linux-kernel, Sanjeev Sharma,
Sanjeev Sharma
In-Reply-To: <54B3D899.5060204@freescale.com>
This patch correct the bad expression while writing the
bit-pattern from software's buffer to hardware registers.
Signed-off-by: Sanjeev Sharma <Sanjeev_Sharma@mentor.com>
---
Changes in v3:
- corrected the expression by introducing tab->fe[i].prop.
drivers/net/ethernet/freescale/gianfar_ethtool.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/freescale/gianfar_ethtool.c b/drivers/net/ethernet/freescale/gianfar_ethtool.c
index 3e1a9c1..fda12fb 100644
--- a/drivers/net/ethernet/freescale/gianfar_ethtool.c
+++ b/drivers/net/ethernet/freescale/gianfar_ethtool.c
@@ -1586,7 +1586,7 @@ static int gfar_write_filer_table(struct gfar_private *priv,
return -EBUSY;
/* Fill regular entries */
- for (; i < MAX_FILER_IDX - 1 && (tab->fe[i].ctrl | tab->fe[i].ctrl);
+ for (; i < MAX_FILER_IDX - 1 && (tab->fe[i].ctrl | tab->fe[i].prop);
i++)
gfar_write_filer(priv, i, tab->fe[i].ctrl, tab->fe[i].prop);
/* Fill the rest with fall-troughs */
--
1.7.11.7
^ permalink raw reply related
* Re: [PATCHv2 net] net: restore lro after device detached from bridge
From: Fan Du @ 2015-02-03 7:08 UTC (permalink / raw)
To: alexander.h.duyck; +Cc: Fan Du, bhutchings, davem, netdev
In-Reply-To: <54CF52D6.90701@redhat.com>
于 2015年02月02日 18:35, Alexander Duyck 写道:
> On 02/01/2015 06:20 PM, Fan Du wrote:
>> 于 2015年01月31日 04:48, Alexander Duyck 写道:
>>> On 01/30/2015 04:33 AM, Fan Du wrote:
>>>> Either detaching a device from bridge or switching a device
>>>> out of FORWARDING state, the original lro feature should
>>>> possibly be enabled for good reason, e.g. hw feature like
>>>> receive side coalescing could come into play.
>>>>
>>>> BEFORE:
>>>> echo 1 > /proc/sys/net/ipv4/conf/ens806f0/forwarding && ethtool -k ens806f0 | grep large
>>>> large-receive-offload: off
>>>>
>>>> echo 0 > /proc/sys/net/ipv4/conf/ens806f0/forwarding && ethtool -k ens806f0 | grep large
>>>> large-receive-offload: off
>>>>
>>>> AFTER:
>>>> echo 1 > /proc/sys/net/ipv4/conf/ens806f0/forwarding && ethtool -k ens806f0 | grep large
>>>> large-receive-offload: off
>>>>
>>>> echo 0 > /proc/sys/net/ipv4/conf/ens806f0/forwarding && ethtool -k ens806f0 | grep large
>>>> large-receive-offload: on
>>>>
>>>> Signed-off-by: Fan Du <fan.du@intel.com>
>>>> Fixes: 0187bdfb0567 ("net: Disable LRO on devices that are forwarding")
>>>
>>
>>> First off this isn't a "fix". This is going to likely break more than
>>> it fixes. The main reason why LRO is disabled is because it can cause
>>> more harm then it helps. Since GRO is available we should err on the
>>> side of caution since enabling LRO/RSC can have undesirable side effects
>>> in a number of cases.
>>
>> I think you are talking about bad scenarios when net device is attached to a bridge.
>> Then what's the good reason user has to pay extra cpu power for using GRO, instead
>> of using hw capable LRO/RSC when this net device is detached from bridge acting as
>> a standalone NIC?
>>
>> Note, SRC is defaulted to *ON* in practice for ALL ixgbe NICs, as same other RSC capable
>> NICs. Attaching net device to a bridge _once_ should not changed its default configuration,
>> moreover it's a subtle change without any message that user won't noticed at all.
> No, RSC only has benefits for IPv4/TCP large packets. However
> historically there have been issues seen w/ small packet performance
> with RSC enabled.
Only when parallel client exceeds 4, gro trumps lro performance on my testbed for small packets.
The difference comes from the fact that TCP RSS hash flows from clients into different NIC queues
for multiple cpu, while RSC engine inside NIC has limit resource compared with that of cpu used by gro.
NICs: 82599EB
server:ipserf -s -B 192.168.5.1
client:iperf -c 192.168.5.1 -i 1 -M 100 -P x
-P Bandwidth/lro on Bandwidth/lro off
gro off gro on
1 2.31 Gbits/sec 947 Mbits/sec
2 3.09 Gbits/sec 1.97 Gbits/sec
3 3.19 Gbits/sec 2.70 Gbits/sec
4 3.16 Gbits/sec 3.39 Gbits/sec
5 3.23 Gbits/sec 3.33 Gbits/sec
6 3.19 Gbits/sec 3.74 Gbits/sec
7 3.18 Gbits/sec 3.88 Gbits/sec
8 3.17 Gbits/sec 3.24 Gbits/sec
9 3.16 Gbits/sec 3.70 Gbits/sec
10 3.15 Gbits/sec 3.76 Gbits/sec
11 3.10 Gbits/sec 4.03 Gbits/sec
12 3.11 Gbits/sec 3.13 Gbits/sec
13 3.12 Gbits/sec 4.12 Gbits/sec
14 3.07 Gbits/sec 4.04 Gbits/sec
15 3.03 Gbits/sec 3.14 Gbits/sec
16 2.99 Gbits/sec 3.93 Gbits/sec
Some have been addressed, however there are still
> other effects such as increasing latency for receive unless the push
> flag is set in the frame.
>
> I still say this patch is not valid, even with your changes. Your
> performance gain doesn't trump the regressions you would be causing on
> other peoples platforms.
>
> I would suggest figuring out why you are seeing issues with routing or
> bridging being enabled and disabled and possibly cleaning up the issue
> via a script rather than trying to modify the kernel to make it take
> care of it for you.
> - Alex
^ permalink raw reply
* Re: [PATCHv2 net] net: restore lro after device detached from bridge
From: Michal Kubecek @ 2015-02-03 6:54 UTC (permalink / raw)
To: Fan Du; +Cc: Alexander Duyck, Fan Du, bhutchings, davem, netdev
In-Reply-To: <54D0326C.2050704@gmail.com>
On Tue, Feb 03, 2015 at 10:29:00AM +0800, Fan Du wrote:
> 于 2015年02月02日 19:15, Michal Kubecek 写道:
> >On Mon, Feb 02, 2015 at 10:20:12AM +0800, Fan Du wrote:
> >
> >>Note, SRC is defaulted to *ON* in practice for ALL ixgbe NICs, as same
> >>other RSC capable NICs.
> >
> >A very bad idea, IMHO. A lot of bug reports resulted from it.
>
> Why are you saying this an idea?? this a fact for all RSC capable NIC
> drivers. search drivers/net/ethernet/ to find more.
I didn't say it's not turned on by default, I just said I consider this
a bad idea. Why? Because the feature is known to break network
communication in common and frequently used scenarios (essentially
whenever received packets may leave the host). When this happens, you
observe strange networking malfunction and unless you know this may be
the cause (or read release notes very carefully), it's very difficult to
identify the cause. Personally, I handled four bug reports of this type
in last three years.
On the other hand, having it turned off when it could be on is only a
performance problem and the communication works. When you are tuning the
performance, you obviously look at the offloading features and check
which could be turned on.
This disbalance (broken communication on one side and slight performance
difference on the other) is why I believe feature like this should not
be turned on by default. Unfortunately it is and we have to deal with
it; that's why dev_disable_lro() is called in certain situations and why
it is propagated down lo lower devices. Turning it back on without
carefully checking that _none_ of the reasons to have it off exists
would do more harm than good.
Michal Kubecek
^ permalink raw reply
* Re: [PATCH] net: rocker: Change netdev names to include slot number
From: Scott Feldman @ 2015-02-03 5:46 UTC (permalink / raw)
To: David Ahern; +Cc: Florian Fainelli, Netdev
In-Reply-To: <54CFF5B3.7070509@gmail.com>
On Mon, Feb 2, 2015 at 2:09 PM, David Ahern <dsahern@gmail.com> wrote:
> On 2/2/15 3:03 PM, Florian Fainelli wrote:
>>
>> Why not? virtio and rocker interfaces are backed by different devices
>> drivers which should allow you to use that to name interfaces
>> differently. In the case of rocker, you would probably want to read the
>> phys_port_id sysfs attribute to name them after their parent switch id
>> too.
>
>
> [root@f21 ~]# cat /sys/devices/virtual/net/sw5p0/phys_port_id
> cat: /sys/devices/virtual/net/sw5p0/phys_port_id: Operation not supported
We should implement .ndo_get_phys_port_id in rocker to return a unique
name for the port, which can then be used in udev script to name
interface. When rocker device is instantiated, a unique switch name
is given on qemu cmd line, for example "sw1". Rocker device could
return that string to the driver, or even combine that string with the
port index to have "sw1p1". The second instantiated rocker device
would give up "sw2px" port names.
A real switch with a physical port than can be split, or when multiple
ports are ganged to form one mega port, letting the device pick a
unique name is probably best. For example, splitting a single
physical 40Gb port into 4 10Gb ports would present 4 kernel
interfaces, but we probably want to use the base port in the naming,
so we'd have something like "sw1p1s1" to mean switch 1, front panel
port 1, split port 1. Letting the device name the ports on current
port configuration of the port will keep driver/kernel simple, and let
udev be the final name chooser.
>
> But in general I guess need to find time to figure out systemd-udev files.
>
> David
^ permalink raw reply
* [PATCH net-next] cxgb4: Add low latency socket busy_poll support
From: Hariprasad Shenai @ 2015-02-03 5:20 UTC (permalink / raw)
To: netdev
Cc: davem, leedom, anish, nirranjan, praveenm, kumaras,
Hariprasad Shenai
cxgb_busy_poll, corresponding to ndo_busy_poll, gets called by the socket
waiting for data.
With busy_poll enabled, improvement is seen in latency numbers as observed by
collecting netperf TCP_RR numbers.
Based on original work by Kumar Sanghvi <kumaras@chelsio.com>
Signed-off-by: Hariprasad Shenai <hariprasad@chelsio.com>
---
drivers/net/ethernet/chelsio/cxgb4/cxgb4.h | 113 +++++++++++++++++++++++
drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 16 +++-
drivers/net/ethernet/chelsio/cxgb4/sge.c | 47 +++++++++-
drivers/net/ethernet/chelsio/cxgb4/t4_values.h | 1 +
4 files changed, 174 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h b/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
index fb6980a..55019c9 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
@@ -476,6 +476,22 @@ struct sge_rspq { /* state for an SGE response queue */
struct adapter *adap;
struct net_device *netdev; /* associated net device */
rspq_handler_t handler;
+#ifdef CONFIG_NET_RX_BUSY_POLL
+#define CXGB_POLL_STATE_IDLE 0
+#define CXGB_POLL_STATE_NAPI BIT(0) /* NAPI owns this poll */
+#define CXGB_POLL_STATE_POLL BIT(1) /* poll owns this poll */
+#define CXGB_POLL_STATE_NAPI_YIELD BIT(2) /* NAPI yielded this poll */
+#define CXGB_POLL_STATE_POLL_YIELD BIT(3) /* poll yielded this poll */
+#define CXGB_POLL_YIELD (CXGB_POLL_STATE_NAPI_YIELD | \
+ CXGB_POLL_STATE_POLL_YIELD)
+#define CXGB_POLL_LOCKED (CXGB_POLL_STATE_NAPI | \
+ CXGB_POLL_STATE_POLL)
+#define CXGB_POLL_USER_PEND (CXGB_POLL_STATE_POLL | \
+ CXGB_POLL_STATE_POLL_YIELD)
+ unsigned int bpoll_state;
+ spinlock_t bpoll_lock; /* lock for busy poll */
+#endif /* CONFIG_NET_RX_BUSY_POLL */
+
};
struct sge_eth_stats { /* Ethernet queue statistics */
@@ -880,6 +896,102 @@ static inline struct adapter *netdev2adap(const struct net_device *dev)
return netdev2pinfo(dev)->adapter;
}
+#ifdef CONFIG_NET_RX_BUSY_POLL
+static inline void cxgb_busy_poll_init_lock(struct sge_rspq *q)
+{
+ spin_lock_init(&q->bpoll_lock);
+ q->bpoll_state = CXGB_POLL_STATE_IDLE;
+}
+
+static inline bool cxgb_poll_lock_napi(struct sge_rspq *q)
+{
+ bool rc = true;
+
+ spin_lock(&q->bpoll_lock);
+ if (q->bpoll_state & CXGB_POLL_LOCKED) {
+ q->bpoll_state |= CXGB_POLL_STATE_NAPI_YIELD;
+ rc = false;
+ } else {
+ q->bpoll_state = CXGB_POLL_STATE_NAPI;
+ }
+ spin_unlock(&q->bpoll_lock);
+ return rc;
+}
+
+static inline bool cxgb_poll_unlock_napi(struct sge_rspq *q)
+{
+ bool rc = false;
+
+ spin_lock(&q->bpoll_lock);
+ if (q->bpoll_state & CXGB_POLL_STATE_POLL_YIELD)
+ rc = true;
+ q->bpoll_state = CXGB_POLL_STATE_IDLE;
+ spin_unlock(&q->bpoll_lock);
+ return rc;
+}
+
+static inline bool cxgb_poll_lock_poll(struct sge_rspq *q)
+{
+ bool rc = true;
+
+ spin_lock_bh(&q->bpoll_lock);
+ if (q->bpoll_state & CXGB_POLL_LOCKED) {
+ q->bpoll_state |= CXGB_POLL_STATE_POLL_YIELD;
+ rc = false;
+ } else {
+ q->bpoll_state |= CXGB_POLL_STATE_POLL;
+ }
+ spin_unlock_bh(&q->bpoll_lock);
+ return rc;
+}
+
+static inline bool cxgb_poll_unlock_poll(struct sge_rspq *q)
+{
+ bool rc = false;
+
+ spin_lock_bh(&q->bpoll_lock);
+ if (q->bpoll_state & CXGB_POLL_STATE_POLL_YIELD)
+ rc = true;
+ q->bpoll_state = CXGB_POLL_STATE_IDLE;
+ spin_unlock_bh(&q->bpoll_lock);
+ return rc;
+}
+
+static inline bool cxgb_poll_busy_polling(struct sge_rspq *q)
+{
+ return q->bpoll_state & CXGB_POLL_USER_PEND;
+}
+#else
+static inline void cxgb_busy_poll_init_lock(struct sge_rspq *q)
+{
+}
+
+static inline bool cxgb_poll_lock_napi(struct sge_rspq *q)
+{
+ return true;
+}
+
+static inline bool cxgb_poll_unlock_napi(struct sge_rspq *q)
+{
+ return false;
+}
+
+static inline bool cxgb_poll_lock_poll(struct sge_rspq *q)
+{
+ return false;
+}
+
+static inline bool cxgb_poll_unlock_poll(struct sge_rspq *q)
+{
+ return false;
+}
+
+static inline bool cxgb_poll_busy_polling(struct sge_rspq *q)
+{
+ return false;
+}
+#endif /* CONFIG_NET_RX_BUSY_POLL */
+
void t4_os_portmod_changed(const struct adapter *adap, int port_id);
void t4_os_link_changed(struct adapter *adap, int port_id, int link_stat);
@@ -908,6 +1020,7 @@ irqreturn_t t4_sge_intr_msix(int irq, void *cookie);
int t4_sge_init(struct adapter *adap);
void t4_sge_start(struct adapter *adap);
void t4_sge_stop(struct adapter *adap);
+int cxgb_busy_poll(struct napi_struct *napi);
extern int dbfifo_int_thresh;
#define for_each_port(adapter, iter) \
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
index 5bf490a..041742b 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
@@ -923,8 +923,14 @@ static void quiesce_rx(struct adapter *adap)
for (i = 0; i < ARRAY_SIZE(adap->sge.ingr_map); i++) {
struct sge_rspq *q = adap->sge.ingr_map[i];
- if (q && q->handler)
+ if (q && q->handler) {
napi_disable(&q->napi);
+ local_bh_disable();
+ while (!cxgb_poll_lock_napi(q))
+ mdelay(1);
+ local_bh_enable();
+ }
+
}
}
@@ -940,8 +946,10 @@ static void enable_rx(struct adapter *adap)
if (!q)
continue;
- if (q->handler)
+ if (q->handler) {
+ cxgb_busy_poll_init_lock(q);
napi_enable(&q->napi);
+ }
/* 0-increment GTS to start the timer and enable interrupts */
t4_write_reg(adap, MYPF_REG(SGE_PF_GTS_A),
SEINTARM_V(q->intr_params) |
@@ -4563,6 +4571,10 @@ static const struct net_device_ops cxgb4_netdev_ops = {
#ifdef CONFIG_NET_POLL_CONTROLLER
.ndo_poll_controller = cxgb_netpoll,
#endif
+#ifdef CONFIG_NET_RX_BUSY_POLL
+ .ndo_busy_poll = cxgb_busy_poll,
+#endif
+
};
void t4_fatal_err(struct adapter *adap)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/sge.c b/drivers/net/ethernet/chelsio/cxgb4/sge.c
index 6191561..7b1a140 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/sge.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/sge.c
@@ -43,6 +43,9 @@
#include <linux/export.h>
#include <net/ipv6.h>
#include <net/tcp.h>
+#ifdef CONFIG_NET_RX_BUSY_POLL
+#include <net/busy_poll.h>
+#endif /* CONFIG_NET_RX_BUSY_POLL */
#include "cxgb4.h"
#include "t4_regs.h"
#include "t4_values.h"
@@ -1720,6 +1723,7 @@ static void do_gro(struct sge_eth_rxq *rxq, const struct pkt_gl *gl,
skb->truesize += skb->data_len;
skb->ip_summed = CHECKSUM_UNNECESSARY;
skb_record_rx_queue(skb, rxq->rspq.idx);
+ skb_mark_napi_id(skb, &rxq->rspq.napi);
if (rxq->rspq.netdev->features & NETIF_F_RXHASH)
skb_set_hash(skb, (__force u32)pkt->rsshdr.hash_val,
PKT_HASH_TYPE_L3);
@@ -1763,6 +1767,7 @@ int t4_ethrx_handler(struct sge_rspq *q, const __be64 *rsp,
csum_ok = pkt->csum_calc && !pkt->err_vec &&
(q->netdev->features & NETIF_F_RXCSUM);
if ((pkt->l2info & htonl(RXF_TCP_F)) &&
+ !(cxgb_poll_busy_polling(q)) &&
(q->netdev->features & NETIF_F_GRO) && csum_ok && !pkt->ip_frag) {
do_gro(rxq, si, pkt);
return 0;
@@ -1801,6 +1806,7 @@ int t4_ethrx_handler(struct sge_rspq *q, const __be64 *rsp,
__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), ntohs(pkt->vlan));
rxq->stats.vlan_ex++;
}
+ skb_mark_napi_id(skb, &q->napi);
netif_receive_skb(skb);
return 0;
}
@@ -1963,6 +1969,38 @@ static int process_responses(struct sge_rspq *q, int budget)
return budget - budget_left;
}
+#ifdef CONFIG_NET_RX_BUSY_POLL
+int cxgb_busy_poll(struct napi_struct *napi)
+{
+ struct sge_rspq *q = container_of(napi, struct sge_rspq, napi);
+ unsigned int params, work_done;
+ u32 val;
+
+ if (!cxgb_poll_lock_poll(q))
+ return LL_FLUSH_BUSY;
+
+ work_done = process_responses(q, 4);
+ params = QINTR_TIMER_IDX(TIMERREG_COUNTER0_X) | QINTR_CNT_EN;
+ q->next_intr_params = params;
+ val = CIDXINC_V(work_done) | SEINTARM_V(params);
+
+ /* If we don't have access to the new User GTS (T5+), use the old
+ * doorbell mechanism; otherwise use the new BAR2 mechanism.
+ */
+ if (unlikely(!q->bar2_addr))
+ t4_write_reg(q->adap, MYPF_REG(SGE_PF_GTS_A),
+ val | INGRESSQID_V((u32)q->cntxt_id));
+ else {
+ writel(val | INGRESSQID_V(q->bar2_qid),
+ q->bar2_addr + SGE_UDB_GTS);
+ wmb();
+ }
+
+ cxgb_poll_unlock_poll(q);
+ return work_done;
+}
+#endif /* CONFIG_NET_RX_BUSY_POLL */
+
/**
* napi_rx_handler - the NAPI handler for Rx processing
* @napi: the napi instance
@@ -1978,9 +2016,13 @@ static int napi_rx_handler(struct napi_struct *napi, int budget)
{
unsigned int params;
struct sge_rspq *q = container_of(napi, struct sge_rspq, napi);
- int work_done = process_responses(q, budget);
+ int work_done = 0;
u32 val;
+ if (!cxgb_poll_lock_napi(q))
+ return work_done;
+
+ work_done = process_responses(q, budget);
if (likely(work_done < budget)) {
int timer_index;
@@ -2018,6 +2060,7 @@ static int napi_rx_handler(struct napi_struct *napi, int budget)
q->bar2_addr + SGE_UDB_GTS);
wmb();
}
+ cxgb_poll_unlock_napi(q);
return work_done;
}
@@ -2341,6 +2384,7 @@ int t4_sge_alloc_rxq(struct adapter *adap, struct sge_rspq *iq, bool fwevtq,
goto err;
netif_napi_add(dev, &iq->napi, napi_rx_handler, 64);
+ napi_hash_add(&iq->napi);
iq->cur_desc = iq->desc;
iq->cidx = 0;
iq->gen = 1;
@@ -2598,6 +2642,7 @@ static void free_rspq_fl(struct adapter *adap, struct sge_rspq *rq,
rq->cntxt_id, fl_id, 0xffff);
dma_free_coherent(adap->pdev_dev, (rq->size + 1) * rq->iqe_len,
rq->desc, rq->phys_addr);
+ napi_hash_del(&rq->napi);
netif_napi_del(&rq->napi);
rq->netdev = NULL;
rq->cntxt_id = rq->abs_id = 0;
diff --git a/drivers/net/ethernet/chelsio/cxgb4/t4_values.h b/drivers/net/ethernet/chelsio/cxgb4/t4_values.h
index a404844..997ec87 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/t4_values.h
+++ b/drivers/net/ethernet/chelsio/cxgb4/t4_values.h
@@ -59,6 +59,7 @@
/* GTS register */
#define SGE_TIMERREGS 6
+#define TIMERREG_COUNTER0_X 0
/* T5 and later support a new BAR2-based doorbell mechanism for Egress Queues.
* The User Doorbells are each 128 bytes in length with a Simple Doorbell at
--
1.7.1
^ permalink raw reply related
* RE: [PATCH net-next 2/8] cxgb4: Added support in debugfs to display tp_la stats
From: Hariprasad S @ 2015-02-03 5:00 UTC (permalink / raw)
To: David Miller
Cc: netdev@vger.kernel.org, Casey Leedom, Anish Bhatt,
Nirranjan Kirubaharan, Praveen Madhavan
In-Reply-To: <20150202.131436.102902508236295371.davem@davemloft.net>
Hi David,
Will send a V2. For now will send only busy_poll.
Thanks,
Hari
________________________________________
From: David Miller [davem@davemloft.net]
Sent: Tuesday, February 03, 2015 2:44 AM
To: Hariprasad S
Cc: netdev@vger.kernel.org; Casey Leedom; Anish Bhatt; Nirranjan Kirubaharan; Praveen Madhavan
Subject: Re: [PATCH net-next 2/8] cxgb4: Added support in debugfs to display tp_la stats
From: Hariprasad Shenai <hariprasad@chelsio.com>
Date: Mon, 2 Feb 2015 20:23:03 +0530
> Signed-off-by: Hariprasad Shenai <hariprasad@chelsio.com>
Ok I've seen enough.
You guys really, truly, abuse debugfs. So I'm putting my foot down now.
Stats like this can be exported through traditional means such as via
ethtool.
^ permalink raw reply
* Re: [PATCHv1 net] xen-netback: stop the guest rx thread after a fatal error
From: David Miller @ 2015-02-03 3:39 UTC (permalink / raw)
To: david.vrabel; +Cc: netdev, xen-devel, ian.campbell, wei.liu2
In-Reply-To: <1422896271-26551-1-git-send-email-david.vrabel@citrix.com>
From: David Vrabel <david.vrabel@citrix.com>
Date: Mon, 2 Feb 2015 16:57:51 +0000
> After commit e9d8b2c2968499c1f96563e6522c56958d5a1d0d (xen-netback:
> disable rogue vif in kthread context), a fatal (protocol) error would
> leave the guest Rx thread spinning, wasting CPU time. Commit
> ecf08d2dbb96d5a4b4bcc53a39e8d29cc8fef02e (xen-netback: reintroduce
> guest Rx stall detection) made this even worse by removing a
> cond_resched() from this path.
>
> Since a fatal error is non-recoverable, just allow the guest Rx thread
> to exit. This requires taking additional refs to the task so the
> thread exiting early is handled safely.
>
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> Reported-by: Julien Grall <julien.grall@linaro.org>
> Tested-by: Julien Grall <julien.grall@linaro.org>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH net] net/mlx4_core: Fix kernel Oops (mem corruption) when working with more than 80 VFs
From: David Miller @ 2015-02-03 3:38 UTC (permalink / raw)
To: amirv; +Cc: netdev, ogerlitz, yevgenyp, jackm
In-Reply-To: <1422883122-29020-1-git-send-email-amirv@mellanox.com>
From: Amir Vadai <amirv@mellanox.com>
Date: Mon, 2 Feb 2015 15:18:42 +0200
> From: Jack Morgenstein <jackm@dev.mellanox.co.il>
>
> Commit de966c592802 (net/mlx4_core: Support more than 64 VFs) was meant to
> allow up to 126 VFs. However, due to leaving MLX4_MFUNC_MAX too low, using
> more than 80 VFs resulted in memory corruptions (and Oopses) when more than
> 80 VFs were requested. In addition, the number of slaves was left too high.
>
> This commit fixes these issues.
>
> Fixes: de966c592802 ("net/mlx4_core: Support more than 64 VFs")
> Signed-off-by: Jack Morgenstein <jackm@dev.mellanox.co.il>
> Signed-off-by: Amir Vadai <amirv@mellanox.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next] pkt_sched: fq: better control of DDOS traffic
From: Eric Dumazet @ 2015-02-03 3:34 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20150202.181834.2201965486446161539.davem@davemloft.net>
On Mon, 2015-02-02 at 18:18 -0800, David Miller wrote:
> Can you document the mask value a little bit more?
>
> For example, I don't understand why "(1024 - 1) << 1" means 1024
> slots just from looking at this change.
>
Sure , will do.
Thats because we reserve low order bit of the sk/hash value,
to make sure the stochastic hash is only applied on non locally
generated traffic.
Thanks
^ permalink raw reply
* Re: [PATCH 0/4] Netfilter/IPVS fixes for net
From: David Miller @ 2015-02-03 3:31 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel, netdev
In-Reply-To: <1422737711-5169-1-git-send-email-pablo@netfilter.org>
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Sat, 31 Jan 2015 21:55:07 +0100
> The following patchset contains Netfilter/IPVS fixes for your net tree,
> they are:
>
> 1) Validate hooks for nf_tables NAT expressions, otherwise users can
> crash the kernel when using them from the wrong hook. We already
> got one user trapped on this when configuring masquerading.
>
> 2) Fix a BUG splat in nf_tables with CONFIG_DEBUG_PREEMPT=y. Reported
> by Andreas Schultz.
>
> 3) Avoid unnecessary reroute of traffic in the local input path
> in IPVS that triggers a crash in in xfrm. Reported by Florian
> Wiessner and fixes by Julian Anastasov.
>
> 4) Fix memory and module refcount leak from the error path of
> nf_tables_newchain().
Pulled, thanks Pablo.
^ permalink raw reply
* Re: [PATCH] net: sctp: Deletion of an unnecessary check before the function call "kfree"
From: David Miller @ 2015-02-03 3:30 UTC (permalink / raw)
To: elfring
Cc: nhorman, vyasevich, linux-sctp, netdev, linux-kernel,
kernel-janitors, julia.lawall
In-Reply-To: <54CD0DCF.9020804@users.sourceforge.net>
From: SF Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 31 Jan 2015 18:15:59 +0100
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sat, 31 Jan 2015 18:10:03 +0100
>
> The kfree() function tests whether its argument is NULL and then
> returns immediately. Thus the test around the call is not needed.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
Applied to net-next, thanks.
^ permalink raw reply
* Re: [PATCH net-next 0/6] ipv6: Add lockless UDP send path
From: David Miller @ 2015-02-03 3:28 UTC (permalink / raw)
To: vyasevich; +Cc: netdev, herbert, hannes, vyasevic
In-Reply-To: <1422718818-21093-1-git-send-email-vyasevic@redhat.com>
From: Vladislav Yasevich <vyasevich@gmail.com>
Date: Sat, 31 Jan 2015 10:40:12 -0500
> This series introduces a lockless UDPv6 send path similar to
> what Herbert Xu did for IPv4 a while ago.
>
> There are some difference from IPv4. IPv6 caching for flow
> label is a bit different, as well as it requires another cork
> cork structure that holds the IPv6 ancillary data.
>
> Please take a look.
This looks good to me, applied, thanks Vlad.
^ permalink raw reply
* Re: rhashtable: Move hash_rnd into bucket_table
From: David Miller @ 2015-02-03 3:26 UTC (permalink / raw)
To: herbert; +Cc: tgraf, netdev
In-Reply-To: <20150202.191956.404010205495035337.davem@davemloft.net>
From: David Miller <davem@davemloft.net>
Date: Mon, 02 Feb 2015 19:19:56 -0800 (PST)
> From: Herbert Xu <herbert@gondor.apana.org.au>
> Date: Sat, 31 Jan 2015 21:21:50 +1100
>
>> Currently hash_rnd is a parameter that users can set. However,
>> no existing users set this parameter. It is also something that
>> people are unlikely to want to set directly since it's just a
>> random number.
>>
>> In preparation for allowing the reseeding/rehashing of rhashtable,
>> this patch moves hash_rnd into bucket_table so that it's now an
>> internal state rather than a parameter.
>>
>> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
>
> Also applied to net-next, thanks a lot.
Actually this and the netfilter change break the build.
I'm reverting all of these rhashtable changes, please fix this stuff
up.
Firstly, the hash_rnd change results in this:
lib/rhashtable.c: In function ‘obj_raw_hashfn’:
lib/rhashtable.c:84:9: warning: passing argument 1 of ‘lockdep_rht_mutex_is_held’ discards ‘const’ qualifier from pointer target type [enabled by default]
lib/rhashtable.c:57:5: note: expected ‘struct rhashtable *’ but argument is of type ‘const struct rhashtable *’
Next, the netfilter change results in:
ERROR: "rhashtable_walk_start" [net/netfilter/nft_hash.ko] undefined!
^ permalink raw reply
* Re: [PATCH net-next 3/4] ethtool: add RX_ALLOC_ORDER to tunable
From: David Miller @ 2015-02-03 3:21 UTC (permalink / raw)
To: _govind; +Cc: netdev, ssujith, benve, edumazet, ben
In-Reply-To: <1422707290-939-4-git-send-email-_govind@gmx.com>
From: Govindarajulu Varadarajan <_govind@gmx.com>
Date: Sat, 31 Jan 2015 17:58:09 +0530
> Signed-off-by: Govindarajulu Varadarajan <_govind@gmx.com>
This is terrible.
You haven't explained what this means.
And to tell you the truth, from what I can tell this tunable is
very specific to how you have implemented RX frags in the enic
driver in this series and won't necessarily translate to how
other drivers manage RX buffers.
You need to actually design this facility properly, understand
what the needs are of other drivers and how this facility
can be relevant for more drivers than your own.
^ permalink raw reply
* Re: Question: should local address be expired when updating PMTU?
From: shengyong @ 2015-02-03 3:21 UTC (permalink / raw)
To: Calvin Owens, Alex Gartrell
Cc: davem, netdev, yangyingling, steffen.klassert, hannes, lvs-devel,
kernel-team
In-Reply-To: <20150203021007.GA1866582@mail.thefacebook.com>
在 2015/2/3 10:10, Calvin Owens 写道:
> On Monday 02/02 at 16:52 -0800, Alex Gartrell wrote:
>> Hello Shengyong,
>>
>>> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
>>> index b2614b2..b80317a 100644
>>> --- a/net/ipv6/route.c
>>> +++ b/net/ipv6/route.c
>>> @@ -1136,6 +1136,9 @@ static void ip6_rt_update_pmtu(struct
>> dst_entry *dst, struct sock *sk,
>>> {
>>> struct rt6_info *rt6 = (struct rt6_info*)dst;
>>>
>>> + if (rt6->rt6i_flags & RTF_LOCAL)
>>> + return;
>>> +
>>> dst_confirm(dst);
>>> if (mtu < dst_mtu(dst) && rt6->rt6i_dst.plen == 128) {
>>> struct net *net = dev_net(dst->dev);
>>>
>>> So is this modification correct? Or how can we avoid such expiring?
>>
>> FWIW, we encountered this problem with IPVS tunneling. Here's a
>> patch done by Calvin (cc'ed) that fixes my attempted fix for this.
>> We're not particularly proud of this...
>>
>> At a high level, I don't think the RTF_LOCAL check was sufficient,
>> but I didn't investigate deeply enough and hopefully Calvin can say
>> why.
>
> I honestly didn't spend much time at all finding the underlying cause
> because it appeared to be fixed upstream: on 3.19-rc5 you get all 3
> expected routes after the last step of my repro below.
Hi,
I do my test on 3.19.0-rc7 just now, it seems it still doesn't solve the
local-addr-expired problem.
I just really
> needed to get this working at the time, and the gross disgusting
> horrible ugly awful [more negative adjectives] patch included below made
> it work.
>
> FWIW, the explanation I wrote down in my notes was:
>
> "The absence of RTF_NONEXTHOP is causing COWs to happen, which are
> always marked as RTF_CACHE. Somehow that's screwing things up in
> rt6_do_redirect()"
>
> That could be BS though, I don't at all remember how I came to that
> conclusion.
>
> (/me resolves to write better notes in the future...)
>
> Here's how to get the weird behavior on 3.10 (+stable):
>
> $ sudo ip addr add local 4444::1 dev lo
> ### Now I have 2 routes in /proc/net/ipv6_route, a local and a non-local
> ### Both have the RTF_NONEXTHOP flag set (0x00200000)
> $ sudo ip route add local 4444::1 dev lo
> ### Now I have 3 routes in /proc/net/ipv6_route to 4444::1
> ### Notice the new route does NOT have the RTF_NONEXTHOP flag set
> $ sudo ip addr del local 4444::1 dev lo
> ### Now I just have the one route I created before
> $ sudo ip addr add local 4444::1 dev lo
> ### And now I have 3 routes again
> $ sudo ping6 4444::1
> [blah blah blah successful ping]
> $ sudo ip addr del local 4444::1 dev lo
> $ sudo ip addr add local 4444::1 dev lo
> ### Still have 3 routes
> $ sudo ip addr del local 4444::1 dev lo
> ### Now I just have my one route yet again
> ### Now, *without the address on lo*, talk to it (it works), then re-add it
> $ ping6 4444::1
> [blah blah blah successful ping]
> $ sudo ip addr add local 4444::1 dev lo
> ### Now I only have 2 routes... WAT!?
> ### Notice the LOCAL (0x80000000) route doesn't have the RTF_NONEXTHOP flag set
Looks like we meet different problems. Here is how I do my test (as well as on 3.10
+stable):
Host only
PC <------------> Virtual Machine
create and send a packet using scapy:
-----------------------------------
| IPv6 (src=PC-addr, dst=VM-addr) |
|---------------------------------|
| ICMPv6 (Packet Too Big) |
|---------------------------------|
| IPv6 (src=VM-addr, dst=VM-addr) |
|---------------------------------|
| ICMPv6 (Neighbor Advertisement) |
-----------------------------------
Then the local-addr is set to expire. After expired, the VM is unreachable from
PC side.
thanks,
Sheng
>
> Thanks,
> Calvin
>
>> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
>> index f14d49b..c607a42 100644
>> --- a/net/ipv6/route.c
>> +++ b/net/ipv6/route.c
>> @@ -1159,18 +1159,18 @@ static void ip6_rt_update_pmtu(struct
>> dst_entry *dst, struct sock *sk,
>> }
>> dst_metric_set(dst, RTAX_MTU, mtu);
>>
>> - /* FACEBOOK HACK: We need to not expire local non-expiring
>> - * routes so that we don't accidentally start blackholing
>> - * ipvs traffic when we happen to use it locally for
>> - * healthchecking (see ip_vs_xmit.c --
>> - * __ip_vs_get_out_rt_v6 invokes update_pmtu if the rt is
>> - * associated with a socket)
>> - * Alex Gartrell <agartrell@fb.com>
>> + /*
>> + * FACEBOOK HACK: Only expire routes that aren't destined for
>> + * the loopback interface.
>> + *
>> + * This prevents the strange route coalescing that happens when
>> + * you add an address to the loopback that had a route that had
>> + * been used when the address didn't exist from getting expired
>> + * and causing packet loss in shiv.
>> */
>> - if (!(rt6->rt6i_flags & RTF_LOCAL) ||
>> - (rt6->rt6i_flags & (RTF_EXPIRES | RTF_CACHE)))
>> - rt6_update_expires(
>> - rt6, net->ipv6.sysctl.ip6_rt_mtu_expires);
>> + if (!(dst->dev->flags & IFF_LOOPBACK))
>> + rt6_update_expires(rt6,
>> + net->ipv6.sysctl.ip6_rt_mtu_expires);
>> }
>> }
>>
>>
>> Cheers,
>> --
>> Alex Gartrell <agartrell@fb.com>
>
> .
>
^ permalink raw reply
* Re: rhashtable: Move hash_rnd into bucket_table
From: David Miller @ 2015-02-03 3:19 UTC (permalink / raw)
To: herbert; +Cc: tgraf, netdev
In-Reply-To: <20150131102150.GA29498@gondor.apana.org.au>
From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Sat, 31 Jan 2015 21:21:50 +1100
> Currently hash_rnd is a parameter that users can set. However,
> no existing users set this parameter. It is also something that
> people are unlikely to want to set directly since it's just a
> random number.
>
> In preparation for allowing the reseeding/rehashing of rhashtable,
> this patch moves hash_rnd into bucket_table so that it's now an
> internal state rather than a parameter.
>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Also applied to net-next, thanks a lot.
^ permalink raw reply
* Re: rhashtable: Fix potential crash on destroy in rhashtable_shrink
From: David Miller @ 2015-02-03 3:19 UTC (permalink / raw)
To: herbert; +Cc: tgraf, netdev
In-Reply-To: <20150131093637.GA29106@gondor.apana.org.au>
From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Sat, 31 Jan 2015 20:36:38 +1100
> The current being_destroyed check in rhashtable_expand is not
> enough since if we start a shrinking process after freeing all
> elements in the table that's also going to crash.
>
> This patch adds a being_destroyed check to the deferred worker
> thread so that we bail out as soon as we take the lock.
>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Applied to net-next
^ permalink raw reply
* Re: netfilter: Use rhashtable walk iterator
From: David Miller @ 2015-02-03 3:19 UTC (permalink / raw)
To: herbert; +Cc: tgraf, ying.xue, kaber, paulmck, netdev, netfilter-devel
In-Reply-To: <20150131043151.GA27524@gondor.apana.org.au>
From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Sat, 31 Jan 2015 15:31:51 +1100
> On Sat, Jan 31, 2015 at 02:13:56PM +1100, Herbert Xu wrote:
>>
>> No problems. Here is a new version of these two patches which
>> hopefully should work on netfilter as well.
>
> And here is the patch for netfilter.
>
> -- >8 --
> This patch gets rid of the manual rhashtable walk in nft_hash
> which touches rhashtable internals that should not be exposed.
> It does so by using the rhashtable iterator primitives.
>
> Note that I'm leaving nft_hash_destroy alone since it's only
> invoked on shutdown and it shouldn't be affected by changes
> to rhashtable internals (or at least not what I'm planning to
> change).
>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Applied to net-next.
^ permalink raw reply
* Re: [PATCH 0/2] rhashtable: Add walk iterator primitives and use them in netlink
From: David Miller @ 2015-02-03 3:19 UTC (permalink / raw)
To: herbert; +Cc: tgraf, ying.xue, kaber, paulmck, netdev, netfilter-devel
In-Reply-To: <20150131031356.GA26781@gondor.apana.org.au>
From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Sat, 31 Jan 2015 14:13:56 +1100
> On Thu, Jan 29, 2015 at 02:42:46PM -0800, David Miller wrote:
>>
>> I'm holding off on this series for now.
>>
>> Let me know if you want me to do something different.
>
> No problems. Here is a new version of these two patches which
> hopefully should work on netfilter as well.
>
> Note that the major functional difference compared to the previous
> one is that resizes are now handled properly and will cause the
> iterator to restart from scratch.
>
> However, as with all existing hash table walker implementations,
> if you remove an element from the chain that we're walking over
> while we're waiting for user-space to give us a new buffer, then
> the walk may miss elements that are still on that chain.
Series applied to net-next.
^ permalink raw reply
* Re: [PATCH net-next v4] Add support of Cavium Liquidio ethernet adapters
From: David Miller @ 2015-02-03 3:00 UTC (permalink / raw)
To: rvatsavayi
Cc: netdev, derek.chickles, satananda.burla, felix.manlunas,
raghu.vatsavayi
In-Reply-To: <1422671378-1300-1-git-send-email-rvatsavayi@caviumnetworks.com>
From: Raghu Vatsavayi <rvatsavayi@caviumnetworks.com>
Date: Fri, 30 Jan 2015 18:29:38 -0800
> create mode 100644 drivers/net/ethernet/cavium/liquidio/cn66xx_device.c
...
> create mode 100644 drivers/net/ethernet/cavium/liquidio/cn68xx_device.c
These two file have several functions which are nearly identical.
Especially the device init register programming sequence.
Please consolidate the common code some more.
THanks.
^ permalink raw reply
* Re: [PATCH v2] net: bluetooth: hci_sock: Use 'const void *' instead of 'void *' for 2nd parameter of hci_test_bit()
From: Chen Gang S @ 2015-02-03 2:59 UTC (permalink / raw)
To: Joe Perches
Cc: marcel, gustavo, johan.hedberg, David S. Miller, linux-bluetooth,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <54D0332B.6050701@sunrus.com.cn>
On 2/3/15 10:32, Chen Gang S wrote:
> On 2/3/15 05:20, Joe Perches wrote:
>> On Tue, 2015-02-03 at 05:14 +0800, Chen Gang S wrote:
>>> hci_test_bit() does not modify 2nd parameter, so it is better to let it
>>> be constant, or may cause build warning. The related warning (with
>>> allmodconfig under xtensa):
>> []
>>> diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
>> []
>>> @@ -46,7 +46,7 @@ struct hci_pinfo {
>>> unsigned short channel;
>>> };
>>>
>>> -static inline int hci_test_bit(int nr, void *addr)
>>> +static inline int hci_test_bit(int nr, const void *addr)
>>> {
>>> return *((__u32 *) addr + (nr >> 5)) & ((__u32) 1 << (nr & 31));
>>> }
>>
>> It's probably better to use const __u32 * here too, but the
>> real thing I wonder is whether or not there's an issue with
>> one of the 2 uses of this function.
>>
>> One of them passes a unsigned long *, the other a u32 *.
>>
>> $ git grep -w hci_test_bit
>> net/bluetooth/hci_sock.c:static inline int hci_test_bit(int nr, void *addr)
>> net/bluetooth/hci_sock.c: if (!hci_test_bit(flt_event, &flt->event_mask))
>> net/bluetooth/hci_sock.c: !hci_test_bit(ocf & HCI_FLT_OCF_BITS,
>> net/bluetooth/hci_sock.c- &hci_sec_filter.ocf_mask[ogf])) &&
>>
>> hci_sec_filter.ocf_mask is __u32
>> but flt->event_mask is unsigned long.
>>
>> Any possible issue here on 64-bit systems?
>>
>
> For me, it can not cause issue on 64-bit systems. hci_test_bit() treats
> 'addr' as "__u32 *", and has to use the pointer to do something.
>
'event_mask' is intended to type cast to "__u32 *" within 'hci_sock.c'.
So for me, "const __u32 *" is better than "const void *" for 2nd
parameter of hci_test_bit().
If what I said above is correct, and also if necessary, I shall patch v3
for it.
Thanks.
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed
^ 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