* Need help with mdiobus_register and phy
From: Timur Tabi @ 2016-10-13 23:15 UTC (permalink / raw)
To: netdev
I need help with a program I've discovered with my driver
(ethernet/qualcomm/emac/) that occurs in the following process:
1. Load the driver
1a. Driver calls mdiobus_register (see emac_phy_config), which calls
mdiobus_scan(), which calls get_phy_device()
1b. get_phy_device() performs MDIO bus operations, which means the bus
and the external phy need to be active.
2. Bring up the interface
3. Bring down the interface. This calls phy_disconnect() which calls
phy_detach() which calls phy_suspend(). This puts the external phy to
sleep.
4. Unload the driver
5. Re-load the driver.
5a. Driver calls mdiobus_register again, but this time the call to
get_phy_device() fails because the external phy is asleep and does not
respond to MDIO transactions.
I don't know what my driver is doing wrong. I'm concerned that
mdiobus_register() is causing MDIO transactions to occur before the rest
of the EMAC has a chance to initialize. I've examined many other
drivers, and I think I'm initializing things in the proper order.
The call to mdiobus_register() occurs before I know what phydev is.
Since I need the phydev in order to talk to the phy driver, I don't see
how I can "wake up" the external phy before calling mdiobus_register().
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply
* [PATCH NET] ethtool: silence warning on bit loss
From: Jesse Brandeburg @ 2016-10-13 23:13 UTC (permalink / raw)
To: netdev
Sparse was complaining when we went to prototype some code
using ethtool_cmd_speed_set and SPEED_100000, which uses
the upper 16 bits of __u32 speed for the first time.
CHECK
...
.../uapi/linux/ethtool.h:123:28: warning:
cast truncates bits from constant value (186a0 becomes 86a0)
The warning is actually bogus, as no bits are really lost, but
we can get rid of the sparse warning with this one small change.
Reported-by: Preethi Banala <preethi.banala@intel.com>
Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
---
include/uapi/linux/ethtool.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index 099a420..8e54723 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -119,8 +119,7 @@ struct ethtool_cmd {
static inline void ethtool_cmd_speed_set(struct ethtool_cmd *ep,
__u32 speed)
{
-
- ep->speed = (__u16)speed;
+ ep->speed = (__u16)(speed & 0xFFFF);
ep->speed_hi = (__u16)(speed >> 16);
}
--
1.8.3.1
^ permalink raw reply related
* [PATCH] net: asix: Avoid looping when the device does not respond
From: Guenter Roeck @ 2016-10-13 23:43 UTC (permalink / raw)
To: David S. Miller; +Cc: linux-usb, netdev, Guenter Roeck
Check answers from USB stack and avoid re-sending the request
multiple times if the device does not respond.
This fixes the following problem, observed with a probably flaky adapter.
[62108.732707] usb 1-3: new high-speed USB device number 5 using xhci_hcd
[62108.914421] usb 1-3: New USB device found, idVendor=0b95, idProduct=7720
[62108.914463] usb 1-3: New USB device strings: Mfr=1, Product=2, SerialNumber=3
[62108.914476] usb 1-3: Product: AX88x72A
[62108.914486] usb 1-3: Manufacturer: ASIX Elec. Corp.
[62108.914495] usb 1-3: SerialNumber: 000001
[62114.109109] asix 1-3:1.0 (unnamed net_device) (uninitialized):
Failed to write reg index 0x0000: -110
[62114.109139] asix 1-3:1.0 (unnamed net_device) (uninitialized):
Failed to send software reset: ffffff92
[62119.109048] asix 1-3:1.0 (unnamed net_device) (uninitialized):
Failed to write reg index 0x0000: -110
...
Since the USB timeout is 5 seconds, and the operation is retried 30 times,
this results in
[62278.180353] INFO: task mtpd:1725 blocked for more than 120 seconds.
[62278.180373] Tainted: G W 3.18.0-13298-g94ace9e #1
[62278.180383] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
...
[62278.180957] kworker/2:0 D 0000000000000000 0 5744 2 0x00000000
[62278.180978] Workqueue: usb_hub_wq hub_event
[62278.181029] ffff880177f833b8 0000000000000046 ffff88017fd00000 ffff88017b126d80
[62278.181048] ffff880177f83fd8 ffff880065a71b60 0000000000013340 ffff880065a71b60
[62278.181065] 0000000000000286 0000000103b1c199 0000000000001388 0000000000000002
[62278.181081] Call Trace:
[62278.181092] [<ffffffff8e0971fd>] ? console_conditional_schedule+0x2c/0x2c
[62278.181105] [<ffffffff8e094f7b>] schedule+0x69/0x6b
[62278.181117] [<ffffffff8e0972e0>] schedule_timeout+0xe3/0x11d
[62278.181133] [<ffffffff8daadb1b>] ? trace_timer_start+0x51/0x51
[62278.181146] [<ffffffff8e095a05>] do_wait_for_common+0x12f/0x16c
[62278.181162] [<ffffffff8da856a7>] ? wake_up_process+0x39/0x39
[62278.181174] [<ffffffff8e095aee>] wait_for_common+0x52/0x6d
[62278.181187] [<ffffffff8e095b3b>] wait_for_completion_timeout+0x13/0x15
[62278.181201] [<ffffffff8de676ce>] usb_start_wait_urb+0x93/0xf1
[62278.181214] [<ffffffff8de6780d>] usb_control_msg+0xe1/0x11d
[62278.181230] [<ffffffffc037d629>] usbnet_write_cmd+0x9c/0xc6 [usbnet]
[62278.181286] [<ffffffffc03af793>] asix_write_cmd+0x4e/0x7e [asix]
[62278.181300] [<ffffffffc03afb41>] asix_set_sw_mii+0x25/0x4e [asix]
[62278.181314] [<ffffffffc03b001d>] asix_mdio_read+0x51/0x109 [asix]
...
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
drivers/net/usb/asix_common.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/usb/asix_common.c b/drivers/net/usb/asix_common.c
index f79eb12c326a..125cff57c759 100644
--- a/drivers/net/usb/asix_common.c
+++ b/drivers/net/usb/asix_common.c
@@ -433,13 +433,13 @@ int asix_mdio_read(struct net_device *netdev, int phy_id, int loc)
mutex_lock(&dev->phy_mutex);
do {
ret = asix_set_sw_mii(dev, 0);
- if (ret == -ENODEV)
+ if (ret == -ENODEV || ret == -ETIMEDOUT)
break;
usleep_range(1000, 1100);
ret = asix_read_cmd(dev, AX_CMD_STATMNGSTS_REG,
0, 0, 1, &smsr, 0);
} while (!(smsr & AX_HOST_EN) && (i++ < 30) && (ret != -ENODEV));
- if (ret == -ENODEV) {
+ if (ret == -ENODEV || ret == -ETIMEDOUT) {
mutex_unlock(&dev->phy_mutex);
return ret;
}
@@ -497,13 +497,13 @@ int asix_mdio_read_nopm(struct net_device *netdev, int phy_id, int loc)
mutex_lock(&dev->phy_mutex);
do {
ret = asix_set_sw_mii(dev, 1);
- if (ret == -ENODEV)
+ if (ret == -ENODEV || ret == -ETIMEDOUT)
break;
usleep_range(1000, 1100);
ret = asix_read_cmd(dev, AX_CMD_STATMNGSTS_REG,
0, 0, 1, &smsr, 1);
} while (!(smsr & AX_HOST_EN) && (i++ < 30) && (ret != -ENODEV));
- if (ret == -ENODEV) {
+ if (ret == -ENODEV || ret == -ETIMEDOUT) {
mutex_unlock(&dev->phy_mutex);
return ret;
}
--
2.5.0
^ permalink raw reply related
* Re: tg3 BUG: spinlock lockup suspected
From: Siva Reddy Kallam @ 2016-10-14 0:55 UTC (permalink / raw)
To: Meelis Roos; +Cc: Linux Netdev List, sparclinux, Linux Kernel list
On Mon, Oct 10, 2016 at 5:21 PM, Siva Reddy Kallam
<siva.kallam@broadcom.com> wrote:
> On Sun, Oct 9, 2016 at 12:35 AM, Meelis Roos <mroos@linux.ee> wrote:
>>> > That did not go well - bisect found the following commit but that does
>>> > not seem to be related at all. So probably the reproducibility is not
>>> > 100% but more random.
>>>
>>> Now I reproduced the bug even with 4.7-rc1 so it is older than 4.7. Will
>>> test further.
>>
>> It gets stranger and stranger - my old 4.7 image worked fine, freshly
>> compiled 4.7 exhibits the same problem.
>>
>> Toolchain has not changed, that I know for sure.
>>
>> What may have changed is kernel .config. My old conf was with whatever I
>> had during 4.7. Then I upgraded to 4.8-rc3 and then 4.8 and selected
>> values for "make oldconfig" new entries. Then went back to 4.7-rc1 and
>> then to 4.7 with this config, answering quiestion about new options when
>> any appeared. Diff is not available since I do not have the old configs
>> archived.
>>
>> Any ideas where to continue from here?
> Probably, You can do fresh system installation if possible.
> Any way, I will try to reproduce with 4.7 and 4.8 kernel versions.
> Will let you know my response in 1-2 days.
We are unable to reproduce with Intel system. we tried with both 4.7
and 4.8 kernel versions.
We are trying to get one SPARC system. We will let you know once we
are done with reproducing with SPARC system.
>>
>>>
>>> >
>>> >
>>> > 4c5773f9f5462dcb372857813918bbfe8c0cdcdd is the first bad commit
>>> > commit 4c5773f9f5462dcb372857813918bbfe8c0cdcdd
>>> > Author: Krzysztof Kozlowski <krzk@kernel.org>
>>> > Date: Sat May 28 11:54:12 2016 +0200
>>> >
>>> > dt-bindings: clock: Add license and reformat Exynos5410 clock IDs
>>> >
>>> > Add license and copyrights (file introduced in 2014) to header with
>>> > Exynos5410 clock IDs. Additionally reformat it to improve readability.
>>> >
>>> > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
>>> > Acked-by: Stephen Boyd <sboyd@codeaurora.org>
>>> > Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>
>>> > Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
>>> >
>>> > :040000 040000 acbd432e11366a8eb8775942bc7b8caa476226e2 08e3a3f98c3d4fa2a93123c3f21b2847c06b4665 M include
>>> >
>>> >
>>> > The whiole bisect log seems to dig around in unrelated places so at best
>>> > it just narrows the window by adding some known-bad data points.
>>> >
>>> > git bisect start
>>> > # good: [523d939ef98fd712632d93a5a2b588e477a7565e] Linux 4.7
>>> > git bisect good 523d939ef98fd712632d93a5a2b588e477a7565e
>>> > # bad: [ef0e1ea8856bed6ff8394d3dfe77f2cab487ecea] Merge tag 'arc-4.8-rc4-fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/vgupta/arc
>>> > git bisect bad ef0e1ea8856bed6ff8394d3dfe77f2cab487ecea
>>> > # good: [e0b3f595d13b3e9ce9cdf53935e7f304c04b5b2b] affs ->d_compare(): don't bother with ->d_inode
>>> > git bisect good e0b3f595d13b3e9ce9cdf53935e7f304c04b5b2b
>>> > # bad: [77a87824ed676ca8ff8482e4157d3adb284fd381] clocksource/drivers/clps_711x: fixup for "ARM: clps711x:
>>> > git bisect bad 77a87824ed676ca8ff8482e4157d3adb284fd381
>>> > # bad: [27acbec338113a75b9d72aeb53149a3538031dda] Merge git://www.linux-watchdog.org/linux-watchdog
>>> > git bisect bad 27acbec338113a75b9d72aeb53149a3538031dda
>>> > # bad: [7f155c702677d057d03b192ce652311de5434697] Merge tag 'nfs-for-4.8-1' of git://git.linux-nfs.org/projects/trondmy/linux-nfs
>>> > git bisect bad 7f155c702677d057d03b192ce652311de5434697
>>> > # good: [797cee982eef9195736afc5e7f3b8f613c41d19a] Merge branch 'stable-4.8' of git://git.infradead.org/users/pcmoore/audit
>>> > git bisect good 797cee982eef9195736afc5e7f3b8f613c41d19a
>>> > # bad: [1056c9bd2702ea1bb79abf9bd1e78c578589d247] Merge tag 'clk-for-linus-4.8' of git://git.kernel.org/pub/scm/linux/kernel/git/clk/linux
>>> > git bisect bad 1056c9bd2702ea1bb79abf9bd1e78c578589d247
>>> > # bad: [1ff435d3571199a799ba6ccfe05544dcd21b9fb3] Merge branch 'clk-st-critical' into clk-next
>>> > git bisect bad 1ff435d3571199a799ba6ccfe05544dcd21b9fb3
>>> > # bad: [0e4504470667d355b53ca3c9802fdd2120c9f946] clk: samsung: exynos5433: Add CLK_IGNORE_UNUSED flag to PCIE device
>>> > git bisect bad 0e4504470667d355b53ca3c9802fdd2120c9f946
>>> > # bad: [880c81b3b6604a004d56b5975c8bed47276e8bf6] clk: samsung: exynos5440: Constify all clock initializers
>>> > git bisect bad 880c81b3b6604a004d56b5975c8bed47276e8bf6
>>> > # bad: [b3a96eed8e84780d300b79b58047ea277ba358b7] clk: samsung: exynos3250: Move platform driver and of_device_id to init section
>>> > git bisect bad b3a96eed8e84780d300b79b58047ea277ba358b7
>>> > # bad: [4528dd8ed477bf202bd33ee48d38d656672d37f8] dt-bindings: clock: Add watchdog and SSS clock IDs to Exynos5410
>>> > git bisect bad 4528dd8ed477bf202bd33ee48d38d656672d37f8
>>> > # bad: [5cd3535a27a7cf8fc4070b499d66e419e7e72b61] dt-bindings: clock: Add PWM and USB clock IDs to Exynos5410
>>> > git bisect bad 5cd3535a27a7cf8fc4070b499d66e419e7e72b61
>>> > # bad: [4c5773f9f5462dcb372857813918bbfe8c0cdcdd] dt-bindings: clock: Add license and reformat Exynos5410 clock IDs
>>> > git bisect bad 4c5773f9f5462dcb372857813918bbfe8c0cdcdd
>>> > # first bad commit: [4c5773f9f5462dcb372857813918bbfe8c0cdcdd] dt-bindings: clock: Add license and reformat Exynos5410 clock IDs
>>> >
>>> >
>>> > >
>>> > > [ 74.123859] tg3.c:v3.137 (May 11, 2014)
>>> > > [ 74.123880] PCI: Enabling device: (0000:00:02.0), cmd 2
>>> > > [ 74.315794] tg3 0000:00:02.0 (unnamed net_device) (uninitialized): Cannot get nvram lock, tg3_nvram_init failed
>>> > > [ 74.656152] tg3 0000:00:02.0 eth0: Tigon3 [partno(none) rev 2003] (PCI:66MHz:64-bit) MAC address 00:03:ba:0a:f3:85
>>> > > [ 74.656160] tg3 0000:00:02.0 eth0: attached PHY is 5704 (10/100/1000Base-T Ethernet) (WireSpeed[1], EEE[0])
>>> > > [ 74.656167] tg3 0000:00:02.0 eth0: RXcsums[1] LinkChgREG[0] MIirq[0] ASF[0] TSOcap[1]
>>> > > [ 74.656172] tg3 0000:00:02.0 eth0: dma_rwctrl[763f0000] dma_mask[32-bit]
>>> > > [ 74.656322] PCI: Enabling device: (0000:00:02.1), cmd 2
>>> > > [ 74.845325] tg3 0000:00:02.1 (unnamed net_device) (uninitialized): Cannot get nvram lock, tg3_nvram_init failed
>>> > > [ 75.184539] tg3 0000:00:02.1 eth1: Tigon3 [partno(none) rev 2003] (PCI:66MHz:64-bit) MAC address 00:03:ba:0a:f3:86
>>> > > [ 75.184546] tg3 0000:00:02.1 eth1: attached PHY is 5704 (10/100/1000Base-T Ethernet) (WireSpeed[1], EEE[0])
>>> > > [ 75.184551] tg3 0000:00:02.1 eth1: RXcsums[1] LinkChgREG[0] MIirq[0] ASF[0] TSOcap[1]
>>> > > [ 75.184557] tg3 0000:00:02.1 eth1: dma_rwctrl[763f0000] dma_mask[32-bit]
>>> > > [ 75.184708] PCI: Enabling device: (0003:00:02.0), cmd 2
>>> > > [ 75.375322] tg3 0003:00:02.0 (unnamed net_device) (uninitialized): Cannot get nvram lock, tg3_nvram_init failed
>>> > > [ 75.714681] tg3 0003:00:02.0 eth2: Tigon3 [partno(none) rev 2003] (PCI:66MHz:64-bit) MAC address 00:03:ba:0a:f3:87
>>> > > [ 75.714688] tg3 0003:00:02.0 eth2: attached PHY is 5704 (10/100/1000Base-T Ethernet) (WireSpeed[1], EEE[0])
>>> > > [ 75.714694] tg3 0003:00:02.0 eth2: RXcsums[1] LinkChgREG[0] MIirq[0] ASF[0] TSOcap[1]
>>> > > [ 75.714699] tg3 0003:00:02.0 eth2: dma_rwctrl[763f0000] dma_mask[32-bit]
>>> > > [ 75.714819] PCI: Enabling device: (0003:00:02.1), cmd 2
>>> > > [ 75.905278] tg3 0003:00:02.1 (unnamed net_device) (uninitialized): Cannot get nvram lock, tg3_nvram_init failed
>>> > > [ 76.244470] tg3 0003:00:02.1 eth3: Tigon3 [partno(none) rev 2003] (PCI:66MHz:64-bit) MAC address 00:03:ba:0a:f3:88
>>> > > [ 76.244477] tg3 0003:00:02.1 eth3: attached PHY is 5704 (10/100/1000Base-T Ethernet) (WireSpeed[1], EEE[0])
>>> > > [ 76.244482] tg3 0003:00:02.1 eth3: RXcsums[1] LinkChgREG[0] MIirq[0] ASF[0] TSOcap[1]
>>> > > [ 76.244488] tg3 0003:00:02.1 eth3: dma_rwctrl[763f0000] dma_mask[32-bit]
>>> > > [ 83.643317] tg3 0000:00:02.0 eth0: No firmware running
>>> > > [...]
>>> > > [ 83.716570] BUG: spinlock lockup suspected on CPU#0, dhclient/1014
>>> > > [ 83.797819] lock: 0xfff000123c8e4a08, .magic: dead4ead, .owner: ip/1001, .owner_cpu: 1
>>> > > [ 83.903130] CPU: 0 PID: 1014 Comm: dhclient Not tainted 4.8.0 #4
>>> > > [ 83.982129] Call Trace:
>>> > > [ 84.014160] [00000000004b7220] spin_dump+0x60/0xa0
>>> > > [ 84.078203] [00000000004b73a0] do_raw_spin_lock+0xa0/0x120
>>> > > [ 84.106344] IPv6: ADDRCONF(NETDEV_UP): eth0: link is not ready
>>> > > [ 84.107193] ip (1001) used greatest stack depth: 2168 bytes left
>>> > > [ 84.306955] [000000000092c0d0] _raw_spin_lock_bh+0x30/0x40
>>> > > [ 84.380188] [00000000100822cc] tg3_get_stats64+0xc/0x80 [tg3]
>>> > > [ 84.456885] [00000000007fac8c] dev_get_stats+0x2c/0xc0
>>> > > [ 84.525506] [000000000081a4e8] dev_seq_printf_stats+0x8/0xe0
>>> > > [ 84.600986] [000000000081a5e4] dev_seq_show+0x24/0x40
>>> > > [ 84.668467] [00000000005cb6c4] seq_read+0x2c4/0x440
>>> > > [ 84.733656] [000000000060b97c] proc_reg_read+0x3c/0x80
>>> > > [ 84.802282] [00000000005a219c] __vfs_read+0x1c/0x140
>>> > > [ 84.868613] [00000000005a2310] vfs_read+0x50/0x100
>>> > > [ 84.932662] [00000000005a265c] SyS_read+0x3c/0xa0
>>> > > [ 84.995573] [00000000004061d4] linux_sparc_syscall32+0x34/0x60
>>> > > [ 85.073748] * CPU[ 0]: TSTATE[00000044f0001a22] TPC[00000000f79a16b0] TNPC[00000000f79a16b4] TASK[dhclient:1014]
>>> > > [ 85.208732] TPC[f79a16b0] O7[f79405c8] I7[0] RPC[0]
>>> > > [ 85.287633] CPU[ 1]: TSTATE[0000004480001605] TPC[00000000004b26f0] TNPC[00000000004d0b0c] TASK[swapper/1:0]
>>> > > [ 85.420338] TPC[trace_hardirqs_off+0x10/0x20] O7[rcu_idle_enter+0x64/0xa0] I7[cpu_startup_entry+0x1b0/0x240] RPC[rest_init+0x178/0x1a0]
>>> > > [ 85.664600] tg3 0000:00:02.0 eth0: Link is up at 100 Mbps, full duplex
>>> > > [ 85.750515] tg3 0000:00:02.0 eth0: Flow control is off for TX and off for RX
>>> > > [ 85.843994] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
>>> > >
>>> > >
>>> >
>>> >
>>>
>>>
>>
>> --
>> Meelis Roos (mroos@linux.ee)
^ permalink raw reply
* [PATCH net-next 1/2] lwtunnel: Add destroy state operation
From: Tom Herbert @ 2016-10-14 0:57 UTC (permalink / raw)
To: davem, netdev, roopa; +Cc: kernel-team
In-Reply-To: <20161014005743.288956-1-tom@herbertland.com>
Users of lwt tunnels may set up some secondary state in build_state
function. Add a corresponding destroy_state function to allow users to
clean up state. This destroy state function is called from lwstate_free.
Also, we now free lwstate using kfree_rcu so user can assume structure
is not freed before rcu.
Signed-off-by: Tom Herbert <tom@herbertland.com>
---
include/net/lwtunnel.h | 7 +++----
net/core/lwtunnel.c | 13 +++++++++++++
2 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/include/net/lwtunnel.h b/include/net/lwtunnel.h
index ea3f80f..119b11b 100644
--- a/include/net/lwtunnel.h
+++ b/include/net/lwtunnel.h
@@ -29,6 +29,7 @@ struct lwtunnel_state {
int (*orig_input)(struct sk_buff *);
int len;
__u16 headroom;
+ struct rcu_head rcu;
__u8 data[0];
};
@@ -43,13 +44,11 @@ struct lwtunnel_encap_ops {
int (*get_encap_size)(struct lwtunnel_state *lwtstate);
int (*cmp_encap)(struct lwtunnel_state *a, struct lwtunnel_state *b);
int (*xmit)(struct sk_buff *skb);
+ void (*destroy_state)(struct lwtunnel_state *lws);
};
#ifdef CONFIG_LWTUNNEL
-static inline void lwtstate_free(struct lwtunnel_state *lws)
-{
- kfree(lws);
-}
+void lwtstate_free(struct lwtunnel_state *lws);
static inline struct lwtunnel_state *
lwtstate_get(struct lwtunnel_state *lws)
diff --git a/net/core/lwtunnel.c b/net/core/lwtunnel.c
index e5f84c2..7401474 100644
--- a/net/core/lwtunnel.c
+++ b/net/core/lwtunnel.c
@@ -130,6 +130,19 @@ int lwtunnel_build_state(struct net_device *dev, u16 encap_type,
}
EXPORT_SYMBOL(lwtunnel_build_state);
+void lwtstate_free(struct lwtunnel_state *lws)
+{
+ const struct lwtunnel_encap_ops *ops = lwtun_encaps[lws->type];
+
+ if (ops->destroy_state) {
+ ops->destroy_state(lws);
+ kfree_rcu(lws, rcu);
+ } else {
+ kfree(lws);
+ }
+}
+EXPORT_SYMBOL(lwtstate_free);
+
int lwtunnel_fill_encap(struct sk_buff *skb, struct lwtunnel_state *lwtstate)
{
const struct lwtunnel_encap_ops *ops;
--
2.9.3
^ permalink raw reply related
* [PATCH net-next 2/2] ila: Cache a route to translated address
From: Tom Herbert @ 2016-10-14 0:57 UTC (permalink / raw)
To: davem, netdev, roopa; +Cc: kernel-team
In-Reply-To: <20161014005743.288956-1-tom@herbertland.com>
Add a dst_cache to ila_lwt structure. This holds a cached route for the
translated address. In ila_output we now perform a route lookup after
translation and if possible (destination in original route is full 128
bits) we set the dst_cache. Subsequent calls to ila_output can then use
the cache to avoid the route lookup.
This eliminates the need to set the gateway on ILA routes as previously
was being done. Now we can do something like:
./ip route add 3333::2000:0:0:2/128 encap ila 2222:0:0:2 \
csum-mode neutral-map dev eth0 ## No via needed!
Signed-off-by: Tom Herbert <tom@herbertland.com>
---
net/ipv6/ila/ila_lwt.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 72 insertions(+), 4 deletions(-)
diff --git a/net/ipv6/ila/ila_lwt.c b/net/ipv6/ila/ila_lwt.c
index e50c27a..df14b00 100644
--- a/net/ipv6/ila/ila_lwt.c
+++ b/net/ipv6/ila/ila_lwt.c
@@ -6,29 +6,80 @@
#include <linux/socket.h>
#include <linux/types.h>
#include <net/checksum.h>
+#include <net/dst_cache.h>
#include <net/ip.h>
#include <net/ip6_fib.h>
+#include <net/ip6_route.h>
#include <net/lwtunnel.h>
#include <net/protocol.h>
#include <uapi/linux/ila.h>
#include "ila.h"
+struct ila_lwt {
+ struct ila_params p;
+ struct dst_cache dst_cache;
+ u32 connected : 1;
+};
+
+static inline struct ila_lwt *ila_lwt_lwtunnel(
+ struct lwtunnel_state *lwstate)
+{
+ return (struct ila_lwt *)lwstate->data;
+}
+
static inline struct ila_params *ila_params_lwtunnel(
struct lwtunnel_state *lwstate)
{
- return (struct ila_params *)lwstate->data;
+ return &((struct ila_lwt *)lwstate->data)->p;
}
static int ila_output(struct net *net, struct sock *sk, struct sk_buff *skb)
{
- struct dst_entry *dst = skb_dst(skb);
+ struct dst_entry *orig_dst = skb_dst(skb);
+ struct ila_lwt *ilwt = ila_lwt_lwtunnel(orig_dst->lwtstate);
+ struct dst_entry *dst;
+ int err = -EINVAL;
if (skb->protocol != htons(ETH_P_IPV6))
goto drop;
- ila_update_ipv6_locator(skb, ila_params_lwtunnel(dst->lwtstate), true);
+ ila_update_ipv6_locator(skb, ila_params_lwtunnel(orig_dst->lwtstate),
+ true);
- return dst->lwtstate->orig_output(net, sk, skb);
+ dst = dst_cache_get(&ilwt->dst_cache);
+ if (unlikely(!dst)) {
+ struct ipv6hdr *ip6h = ipv6_hdr(skb);
+ struct flowi6 fl6;
+
+ /* Lookup a route for the new destination. Take into
+ * account that the base route may already have a gateway.
+ */
+
+ memset(&fl6, 0, sizeof(fl6));
+ fl6.flowi6_oif = orig_dst->dev->ifindex;
+ fl6.flowi6_iif = LOOPBACK_IFINDEX;
+ fl6.daddr = *rt6_nexthop((struct rt6_info *)orig_dst,
+ &ip6h->daddr);
+
+ dst = ip6_route_output(net, NULL, &fl6);
+ if (dst->error) {
+ err = -EHOSTUNREACH;
+ dst_release(dst);
+ goto drop;
+ }
+
+ dst = xfrm_lookup(net, dst, flowi6_to_flowi(&fl6), NULL, 0);
+ if (IS_ERR(dst)) {
+ err = PTR_ERR(dst);
+ goto drop;
+ }
+
+ if (ilwt->connected)
+ dst_cache_set_ip6(&ilwt->dst_cache, dst, &fl6.saddr);
+ }
+
+ skb_dst_set(skb, dst);
+ return dst_output(dev_net(skb_dst(skb)->dev), sk, skb);
drop:
kfree_skb(skb);
@@ -60,6 +111,7 @@ static int ila_build_state(struct net_device *dev, struct nlattr *nla,
unsigned int family, const void *cfg,
struct lwtunnel_state **ts)
{
+ struct ila_lwt *ilwt;
struct ila_params *p;
struct nlattr *tb[ILA_ATTR_MAX + 1];
size_t encap_len = sizeof(*p);
@@ -99,6 +151,13 @@ static int ila_build_state(struct net_device *dev, struct nlattr *nla,
if (!newts)
return -ENOMEM;
+ ilwt = ila_lwt_lwtunnel(newts);
+ ret = dst_cache_init(&ilwt->dst_cache, GFP_ATOMIC);
+ if (ret) {
+ kfree(newts);
+ return ret;
+ }
+
newts->len = encap_len;
p = ila_params_lwtunnel(newts);
@@ -120,11 +179,19 @@ static int ila_build_state(struct net_device *dev, struct nlattr *nla,
newts->flags |= LWTUNNEL_STATE_OUTPUT_REDIRECT |
LWTUNNEL_STATE_INPUT_REDIRECT;
+ if (cfg6->fc_dst_len == sizeof(struct in6_addr))
+ ilwt->connected = 1;
+
*ts = newts;
return 0;
}
+static void ila_destroy_state(struct lwtunnel_state *lwt)
+{
+ dst_cache_destroy(&ila_lwt_lwtunnel(lwt)->dst_cache);
+}
+
static int ila_fill_encap_info(struct sk_buff *skb,
struct lwtunnel_state *lwtstate)
{
@@ -159,6 +226,7 @@ static int ila_encap_cmp(struct lwtunnel_state *a, struct lwtunnel_state *b)
static const struct lwtunnel_encap_ops ila_encap_ops = {
.build_state = ila_build_state,
+ .destroy_state = ila_destroy_state,
.output = ila_output,
.input = ila_input,
.fill_encap = ila_fill_encap_info,
--
2.9.3
^ permalink raw reply related
* [PATCH net-next 0/2] ila: Cache a route in ILA lwt structure
From: Tom Herbert @ 2016-10-14 0:57 UTC (permalink / raw)
To: davem, netdev, roopa; +Cc: kernel-team
Add a dst_cache to ila_lwt structure. This holds a cached route for the
translated address. In ila_output we now perform a route lookup after
translation and if possible (destination in original route is full 128
bits) we set the dst_cache. Subsequent calls to ila_output can then use
the cache to avoid the route lookup.
This eliminates the need to set the gateway on ILA routes as previously
was being done. Now we can do something like:
./ip route add 3333::2000:0:0:2/128 encap ila 2222:0:0:2 \
csum-mode neutral-map dev eth0 ## No via needed!
Also, add destroy_state to lwt ops. We need this do destroy the
dst_cache.
Tested:
Running 200 TCP_RR streams:
Baseline, no ILA
1730716 tps
102/170/313 50/90/99% latencies
88.11 CPU utilization
Using ILA in both directions
1680428 tps
105/176/325 50/90/99% latencies
88.16 CPU utilization
Tom Herbert (2):
lwtunnel: Add destroy state operation
ila: Cache a route to translated address
include/net/lwtunnel.h | 7 ++---
net/core/lwtunnel.c | 13 +++++++++
net/ipv6/ila/ila_lwt.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++---
3 files changed, 88 insertions(+), 8 deletions(-)
--
2.9.3
^ permalink raw reply
* bug in ixgbe_atr
From: Sowmini Varadhan @ 2016-10-14 1:44 UTC (permalink / raw)
To: alexander.h.duyck, netdev
When I was playing around with TPACKET_V2, I think I ran
into a bug in ixgbe_atr(): if I get here with an
sk_buff that has, e.g., just 14 bytes in the header, then
the skb_network_header is invalid. I found my kernel sometimes
wandering off into ipv6_find_hdr() in an attempt to get the
l4_proto, and then complaining that "IPv6 header not found\n"
(this was an ipv4 packet).
I think we want to use skb_header_pointer in ixgbe_atr to get
the network header itself.. I tried the patch below, and it
works for simple (non-vlan, basic) ethernet header, but probably
needs more refinement to work for more complex encapsulations?
And other drivers may need a similar fix too, I've not checked yet.
--Sowmini
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index a244d9a..be453c6 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -7627,6 +7627,10 @@ static void ixgbe_atr(struct ixgbe_ring *ring,
struct iphdr *ipv4;
struct ipv6hdr *ipv6;
} hdr;
+ union {
+ struct iphdr ipv4;
+ struct ipv6hdr ipv6;
+ } ip_hdr;
struct tcphdr *th;
unsigned int hlen;
struct sk_buff *skb;
@@ -7667,13 +7671,15 @@ static void ixgbe_atr(struct ixgbe_ring *ring,
}
/* Currently only IPv4/IPv6 with TCP is supported */
- switch (hdr.ipv4->version) {
- case IPVERSION:
+ switch (ntohs(first->protocol)) {
+ case ETH_P_IP:
+ skb_header_pointer(skb, ETH_HLEN, sizeof (struct iphdr),
+ &ip_hdr);
/* access ihl as u8 to avoid unaligned access on ia64 */
- hlen = (hdr.network[0] & 0x0F) << 2;
- l4_proto = hdr.ipv4->protocol;
+ hlen = ip_hdr.ipv4.ihl << 2;
+ l4_proto = ip_hdr.ipv4.protocol;
break;
- case 6:
+ case ETH_P_IPV6:
hlen = hdr.network - skb->data;
l4_proto = ipv6_find_hdr(skb, &hlen, IPPROTO_TCP, NULL, NULL);
hlen -= hdr.network - skb->data;
^ permalink raw reply related
* RE: bug in ixgbe_atr
From: Duyck, Alexander H @ 2016-10-14 2:06 UTC (permalink / raw)
To: Sowmini Varadhan, netdev@vger.kernel.org
In-Reply-To: <20161014014422.GA14253@oracle.com>
> -----Original Message-----
> From: Sowmini Varadhan [mailto:sowmini.varadhan@oracle.com]
> Sent: Thursday, October 13, 2016 6:44 PM
> To: Duyck, Alexander H <alexander.h.duyck@intel.com>;
> netdev@vger.kernel.org
> Subject: bug in ixgbe_atr
>
> When I was playing around with TPACKET_V2, I think I ran into a bug in
> ixgbe_atr(): if I get here with an sk_buff that has, e.g., just 14 bytes in the
> header, then the skb_network_header is invalid. I found my kernel sometimes
> wandering off into ipv6_find_hdr() in an attempt to get the l4_proto, and then
> complaining that "IPv6 header not found\n"
> (this was an ipv4 packet).
>
> I think we want to use skb_header_pointer in ixgbe_atr to get the network
> header itself.. I tried the patch below, and it works for simple (non-vlan, basic)
> ethernet header, but probably needs more refinement to work for more
> complex encapsulations?
>
> And other drivers may need a similar fix too, I've not checked yet.
>
> --Sowmini
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index a244d9a..be453c6 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -7627,6 +7627,10 @@ static void ixgbe_atr(struct ixgbe_ring *ring,
> struct iphdr *ipv4;
> struct ipv6hdr *ipv6;
> } hdr;
> + union {
> + struct iphdr ipv4;
> + struct ipv6hdr ipv6;
> + } ip_hdr;
> struct tcphdr *th;
> unsigned int hlen;
> struct sk_buff *skb;
> @@ -7667,13 +7671,15 @@ static void ixgbe_atr(struct ixgbe_ring *ring,
> }
>
> /* Currently only IPv4/IPv6 with TCP is supported */
> - switch (hdr.ipv4->version) {
> - case IPVERSION:
> + switch (ntohs(first->protocol)) {
> + case ETH_P_IP:
> + skb_header_pointer(skb, ETH_HLEN, sizeof (struct iphdr),
> + &ip_hdr);
> /* access ihl as u8 to avoid unaligned access on ia64 */
> - hlen = (hdr.network[0] & 0x0F) << 2;
> - l4_proto = hdr.ipv4->protocol;
> + hlen = ip_hdr.ipv4.ihl << 2;
> + l4_proto = ip_hdr.ipv4.protocol;
> break;
> - case 6:
> + case ETH_P_IPV6:
> hlen = hdr.network - skb->data;
> l4_proto = ipv6_find_hdr(skb, &hlen, IPPROTO_TCP, NULL,
> NULL);
> hlen -= hdr.network - skb->data;
The problem is this will break other stuff, for example I have seen the ihl access actually cause problems with unaligned accesses as some architectures decide to pull it as a u32 and then mask it.
My advice would be to keep this simple. Add a check to make sure we have room for at least skb_headlen(skb) - 40 >= hrd.raw - skb->data. Messing with the protocol bits will break stuff since there is support for tunneling also floating around in here now.
I believe we are planning on dropping this code in favor of ndo_rx_flow_steer in the future. If we do that then the whole problem becomes moot.
- Alex
^ permalink raw reply
* linux-next: manual merge of the akpm tree with the net tree
From: Stephen Rothwell @ 2016-10-14 2:12 UTC (permalink / raw)
To: Andrew Morton, David Miller, Networking
Cc: linux-next, linux-kernel, Tom Herbert, Saeed Mahameed
Hi Andrew,
Today's linux-next merge of the akpm tree got a conflict in:
include/linux/mlx5/device.h
between commit:
b8a4ddb2e8f4 ("net/mlx5: Add MLX5_ARRAY_SET64 to fix BUILD_BUG_ON")
from the net tree and patch:
"include/linux/mlx5/device.h: kill BUILD_BUG_ON()s"
from the akpm tree.
I fixed it up (I just dropped the akpm tree patch for today) and can
carry the fix as necessary. This is now fixed as far as linux-next is
concerned, but any non trivial conflicts should be mentioned to your
upstream maintainer when your tree is submitted for merging. You may
also want to consider cooperating with the maintainer of the conflicting
tree to minimise any particularly complex conflicts.
--
Cheers,
Stephen Rothwell
^ permalink raw reply
* [PATCH net 2/5] net/ncsi: Split out logic for ncsi_dev_state_suspend_select
From: Gavin Shan @ 2016-10-14 2:53 UTC (permalink / raw)
To: netdev; +Cc: davem, joel, Gavin Shan
In-Reply-To: <1476413614-24586-1-git-send-email-gwshan@linux.vnet.ibm.com>
This splits out the code that handles ncsi_dev_state_suspend_select
so that we can add more code to the handler in subsequent patch.
Apart from adding a error tag to reuse the code in error path,
no logical changes introduced.
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
net/ncsi/ncsi-manage.c | 38 +++++++++++++++++++++++++-------------
1 file changed, 25 insertions(+), 13 deletions(-)
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 1bc96dc..5758a26 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -540,21 +540,30 @@ static void ncsi_suspend_channel(struct ncsi_dev_priv *ndp)
nd->state = ncsi_dev_state_suspend_select;
/* Fall through */
case ncsi_dev_state_suspend_select:
+ ndp->pending_req_num = 1;
+
+ nca.type = NCSI_PKT_CMD_SP;
+ nca.package = np->id;
+ nca.channel = NCSI_RESERVED_CHANNEL;
+ if (ndp->flags & NCSI_DEV_HWA)
+ nca.bytes[0] = 0;
+ else
+ nca.bytes[0] = 1;
+
+ nd->state = ncsi_dev_state_suspend_dcnt;
+
+ ret = ncsi_xmit_cmd(&nca);
+ if (ret)
+ goto error;
+
+ break;
case ncsi_dev_state_suspend_dcnt:
case ncsi_dev_state_suspend_dc:
case ncsi_dev_state_suspend_deselect:
ndp->pending_req_num = 1;
nca.package = np->id;
- if (nd->state == ncsi_dev_state_suspend_select) {
- nca.type = NCSI_PKT_CMD_SP;
- nca.channel = NCSI_RESERVED_CHANNEL;
- if (ndp->flags & NCSI_DEV_HWA)
- nca.bytes[0] = 0;
- else
- nca.bytes[0] = 1;
- nd->state = ncsi_dev_state_suspend_dcnt;
- } else if (nd->state == ncsi_dev_state_suspend_dcnt) {
+ if (nd->state == ncsi_dev_state_suspend_dcnt) {
nca.type = NCSI_PKT_CMD_DCNT;
nca.channel = nc->id;
nd->state = ncsi_dev_state_suspend_dc;
@@ -570,10 +579,8 @@ static void ncsi_suspend_channel(struct ncsi_dev_priv *ndp)
}
ret = ncsi_xmit_cmd(&nca);
- if (ret) {
- nd->state = ncsi_dev_state_functional;
- return;
- }
+ if (ret)
+ goto error;
break;
case ncsi_dev_state_suspend_done:
@@ -587,6 +594,11 @@ static void ncsi_suspend_channel(struct ncsi_dev_priv *ndp)
netdev_warn(nd->dev, "Wrong NCSI state 0x%x in suspend\n",
nd->state);
}
+
+ return;
+
+error:
+ nd->state = ncsi_dev_state_functional;
}
static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
--
2.1.0
^ permalink raw reply related
* [PATCH net 1/5] net/ncsi: Not fetch active package and channel again
From: Gavin Shan @ 2016-10-14 2:53 UTC (permalink / raw)
To: netdev; +Cc: davem, joel, Gavin Shan
In-Reply-To: <1476413614-24586-1-git-send-email-gwshan@linux.vnet.ibm.com>
In ncsi_suspend_channel(), we fetch the active package and
channel to local variables for twice. It's unnecessary. This
drops one of them. No functional changes introduced.
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
net/ncsi/ncsi-manage.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 5e509e5..1bc96dc 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -545,8 +545,6 @@ static void ncsi_suspend_channel(struct ncsi_dev_priv *ndp)
case ncsi_dev_state_suspend_deselect:
ndp->pending_req_num = 1;
- np = ndp->active_package;
- nc = ndp->active_channel;
nca.package = np->id;
if (nd->state == ncsi_dev_state_suspend_select) {
nca.type = NCSI_PKT_CMD_SP;
--
2.1.0
^ permalink raw reply related
* [PATCH net 3/5] net/ncsi: Fix stale link state of inactive channels on failover
From: Gavin Shan @ 2016-10-14 2:53 UTC (permalink / raw)
To: netdev; +Cc: davem, joel, Gavin Shan
In-Reply-To: <1476413614-24586-1-git-send-email-gwshan@linux.vnet.ibm.com>
The issue was found on BCM5718 which has two NCSI channels in one
package: C0 and C1. Both of them are connected to different LANs,
means they are in link-up state and C0 is chosen as the active
one until resetting BCM5718 happens as below.
Resetting BCM5718 results in LSC (Link State Change) AEN packet
received on C0, meaning LSC AEN is missed on C1. When LSC AEN packet
received on C0 to report link-down, it fails over to C1 because C1
is in link-up state as software can see. However, C1 is in link-down
state in hardware. It means the link state is out of synchronization
between hardware and software, resulting in inappropriate channel (C1)
selected as active one.
This resolves the issue by sending separate GLS (Get Link Status)
commands to all channels in the package before trying to do failover.
The last link state on all channels in the package is retrieved. With
it, C0 is selected as active one as expected.
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
net/ncsi/internal.h | 1 +
net/ncsi/ncsi-manage.c | 20 +++++++++++++++++++-
2 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
index 13290a7..eac4858 100644
--- a/net/ncsi/internal.h
+++ b/net/ncsi/internal.h
@@ -246,6 +246,7 @@ enum {
ncsi_dev_state_config_gls,
ncsi_dev_state_config_done,
ncsi_dev_state_suspend_select = 0x0401,
+ ncsi_dev_state_suspend_gls,
ncsi_dev_state_suspend_dcnt,
ncsi_dev_state_suspend_dc,
ncsi_dev_state_suspend_deselect,
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 5758a26..e959979 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -550,13 +550,31 @@ static void ncsi_suspend_channel(struct ncsi_dev_priv *ndp)
else
nca.bytes[0] = 1;
- nd->state = ncsi_dev_state_suspend_dcnt;
+ if (ndp->flags & NCSI_DEV_RESHUFFLE)
+ nd->state = ncsi_dev_state_suspend_gls;
+ else
+ nd->state = ncsi_dev_state_suspend_dcnt;
ret = ncsi_xmit_cmd(&nca);
if (ret)
goto error;
break;
+ case ncsi_dev_state_suspend_gls:
+ ndp->pending_req_num = np->channel_num;
+
+ nca.type = NCSI_PKT_CMD_GLS;
+ nca.package = np->id;
+ nd->state = ncsi_dev_state_suspend_dcnt;
+
+ NCSI_FOR_EACH_CHANNEL(np, nc) {
+ nca.channel = nc->id;
+ ret = ncsi_xmit_cmd(&nca);
+ if (ret)
+ goto error;
+ }
+
+ break;
case ncsi_dev_state_suspend_dcnt:
case ncsi_dev_state_suspend_dc:
case ncsi_dev_state_suspend_deselect:
--
2.1.0
^ permalink raw reply related
* [PATCH net 5/5] net/ncsi: Improve HNCDSC AEN handler
From: Gavin Shan @ 2016-10-14 2:53 UTC (permalink / raw)
To: netdev; +Cc: davem, joel, Gavin Shan
In-Reply-To: <1476413614-24586-1-git-send-email-gwshan@linux.vnet.ibm.com>
This improves AEN handler for Host Network Controller Driver Status
Change (HNCDSC):
* The channel's lock should be hold when accessing its state.
* Do failover when host driver isn't ready.
* Configure channel when host driver becomes ready.
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
net/ncsi/ncsi-aen.c | 18 +++++++++++++++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/net/ncsi/ncsi-aen.c b/net/ncsi/ncsi-aen.c
index b41a661..6898e72 100644
--- a/net/ncsi/ncsi-aen.c
+++ b/net/ncsi/ncsi-aen.c
@@ -141,23 +141,35 @@ static int ncsi_aen_handler_hncdsc(struct ncsi_dev_priv *ndp,
return -ENODEV;
/* If the channel is active one, we need reconfigure it */
+ spin_lock_irqsave(&nc->lock, flags);
ncm = &nc->modes[NCSI_MODE_LINK];
hncdsc = (struct ncsi_aen_hncdsc_pkt *)h;
ncm->data[3] = ntohl(hncdsc->status);
if (!list_empty(&nc->link) ||
- nc->state != NCSI_CHANNEL_ACTIVE ||
- (ncm->data[3] & 0x1))
+ nc->state != NCSI_CHANNEL_ACTIVE) {
+ spin_unlock_irqrestore(&nc->lock, flags);
return 0;
+ }
- if (ndp->flags & NCSI_DEV_HWA)
+ spin_unlock_irqrestore(&nc->lock, flags);
+ if (!(ndp->flags & NCSI_DEV_HWA) && !(ncm->data[3] & 0x1))
ndp->flags |= NCSI_DEV_RESHUFFLE;
/* If this channel is the active one and the link doesn't
* work, we have to choose another channel to be active one.
* The logic here is exactly similar to what we do when link
* is down on the active channel.
+ *
+ * On the other hand, we need configure it when host driver
+ * state on the active channel becomes ready.
*/
ncsi_stop_channel_monitor(nc);
+
+ spin_lock_irqsave(&nc->lock, flags);
+ nc->state = (ncm->data[3] & 0x1) ? NCSI_CHANNEL_INACTIVE :
+ NCSI_CHANNEL_ACTIVE;
+ spin_unlock_irqrestore(&nc->lock, flags);
+
spin_lock_irqsave(&ndp->lock, flags);
list_add_tail_rcu(&nc->link, &ndp->channel_queue);
spin_unlock_irqrestore(&ndp->lock, flags);
--
2.1.0
^ permalink raw reply related
* [PATCH net 4/5] net/ncsi: Choose hot channel as active one if necessary
From: Gavin Shan @ 2016-10-14 2:53 UTC (permalink / raw)
To: netdev; +Cc: davem, joel, Gavin Shan
In-Reply-To: <1476413614-24586-1-git-send-email-gwshan@linux.vnet.ibm.com>
The issue was found on BCM5718 which has two NCSI channels in one
package: C0 and C1. C0 is in link-up state while C1 is in link-down
state. C0 is chosen as active channel until unplugging and plugging
C0's cable: On unplugging C0's cable, LSC (Link State Change) AEN
packet received on C0 to report link-down event. After that, C1 is
chosen as active channel. LSC AEN for link-up event is lost on C0
when plugging C0's cable back. We lose the network even C0 is usable.
This resolves the issue by recording the (hot) channel that was ever
chosen as active one. The hot channel is chosen to be active one
if none of available channels in link-up state. With this, C0 is still
the active one after unplugging C0's cable. LSC AEN packet received
on C0 when plugging its cable back.
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
net/ncsi/internal.h | 1 +
net/ncsi/ncsi-manage.c | 22 +++++++++++++++++++---
2 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
index eac4858..1308a56 100644
--- a/net/ncsi/internal.h
+++ b/net/ncsi/internal.h
@@ -265,6 +265,7 @@ struct ncsi_dev_priv {
#endif
unsigned int package_num; /* Number of packages */
struct list_head packages; /* List of packages */
+ struct ncsi_channel *hot_channel; /* Channel was ever active */
struct ncsi_request requests[256]; /* Request table */
unsigned int request_id; /* Last used request ID */
#define NCSI_REQ_START_IDX 1
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index e959979..cccedcf 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -625,6 +625,7 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
struct net_device *dev = nd->dev;
struct ncsi_package *np = ndp->active_package;
struct ncsi_channel *nc = ndp->active_channel;
+ struct ncsi_channel *hot_nc = NULL;
struct ncsi_cmd_arg nca;
unsigned char index;
unsigned long flags;
@@ -730,12 +731,20 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
break;
case ncsi_dev_state_config_done:
spin_lock_irqsave(&nc->lock, flags);
- if (nc->modes[NCSI_MODE_LINK].data[2] & 0x1)
+ if (nc->modes[NCSI_MODE_LINK].data[2] & 0x1) {
+ hot_nc = nc;
nc->state = NCSI_CHANNEL_ACTIVE;
- else
+ } else {
+ hot_nc = NULL;
nc->state = NCSI_CHANNEL_INACTIVE;
+ }
spin_unlock_irqrestore(&nc->lock, flags);
+ /* Update the hot channel */
+ spin_lock_irqsave(&ndp->lock, flags);
+ ndp->hot_channel = hot_nc;
+ spin_unlock_irqrestore(&ndp->lock, flags);
+
ncsi_start_channel_monitor(nc);
ncsi_process_next_channel(ndp);
break;
@@ -753,10 +762,14 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
static int ncsi_choose_active_channel(struct ncsi_dev_priv *ndp)
{
struct ncsi_package *np;
- struct ncsi_channel *nc, *found;
+ struct ncsi_channel *nc, *found, *hot_nc;
struct ncsi_channel_mode *ncm;
unsigned long flags;
+ spin_lock_irqsave(&ndp->lock, flags);
+ hot_nc = ndp->hot_channel;
+ spin_unlock_irqrestore(&ndp->lock, flags);
+
/* The search is done once an inactive channel with up
* link is found.
*/
@@ -774,6 +787,9 @@ static int ncsi_choose_active_channel(struct ncsi_dev_priv *ndp)
if (!found)
found = nc;
+ if (nc == hot_nc)
+ found = nc;
+
ncm = &nc->modes[NCSI_MODE_LINK];
if (ncm->data[2] & 0x1) {
spin_unlock_irqrestore(&nc->lock, flags);
--
2.1.0
^ permalink raw reply related
* [PATCH net 0/5] net/ncsi: More bug fixes
From: Gavin Shan @ 2016-10-14 2:53 UTC (permalink / raw)
To: netdev; +Cc: davem, joel, Gavin Shan
This series fixes 2 issues that were found during NCSI's availability
testing on BCM5718 and improves HNCDSC AEN handler:
* PATCH[1] and PATCH[2] refactors the code so that minimal code
change is put to PATCH[3].
* PATCH[3] fixes the NCSI channel's stale link state before doing
failover.
* PATCH[4] chooses the hot channel, which was ever chosen as active
channel, when the available channels are all in link-down state.
* PATCH[5] improves Host Network Controller Driver Status Change
(HNCDSC) AEN handler
Gavin Shan (5):
net/ncsi: Not fetch active package and channel again
net/ncsi: Split out logic for ncsi_dev_state_suspend_select
net/ncsi: Fix stale link state of inactive channels on failover
net/ncsi: Choose hot channel as active one if necessary
net/ncsi: Improve HNCDSC AEN handler
net/ncsi/internal.h | 2 ++
net/ncsi/ncsi-aen.c | 18 ++++++++++--
net/ncsi/ncsi-manage.c | 80 ++++++++++++++++++++++++++++++++++++++------------
3 files changed, 79 insertions(+), 21 deletions(-)
--
2.1.0
^ permalink raw reply
* Re: [PATCH] net: limit a number of namespaces which can be cleaned up concurrently
From: Eric W. Biederman @ 2016-10-14 3:06 UTC (permalink / raw)
To: Andrei Vagin; +Cc: Andrei Vagin, netdev, containers, David S. Miller
In-Reply-To: <20161013204405.GA19836@outlook.office365.com>
Andrei Vagin <avagin@virtuozzo.com> writes:
> On Thu, Oct 13, 2016 at 10:49:38AM -0500, Eric W. Biederman wrote:
>> Andrei Vagin <avagin@openvz.org> writes:
>>
>> > From: Andrey Vagin <avagin@openvz.org>
>> >
>> > The operation of destroying netns is heavy and it is executed under
>> > net_mutex. If many namespaces are destroyed concurrently, net_mutex can
>> > be locked for a long time. It is impossible to create a new netns during
>> > this period of time.
>>
>> This may be the right approach or at least the right approach to bound
>> net_mutex hold times but I have to take exception to calling network
>> namespace cleanup heavy.
>>
>> The only particularly time consuming operation I have ever found are calls to
>> synchronize_rcu/sycrhonize_sched/synchronize_net.
>
> I booted the kernel with maxcpus=1, in this case these functions work
> very fast and the problem is there any way.
>
> Accoding to perf, we spend a lot of time in kobject_uevent:
>
> - 99.96% 0.00% kworker/u4:1 [kernel.kallsyms] [k] unregister_netdevice_many ▒
> - unregister_netdevice_many ◆
> - 99.95% rollback_registered_many ▒
> - 99.64% netdev_unregister_kobject ▒
> - 33.43% netdev_queue_update_kobjects ▒
> - 33.40% kobject_put ▒
> - kobject_release ▒
> + 33.37% kobject_uevent ▒
> + 0.03% kobject_del ▒
> + 0.03% sysfs_remove_group ▒
> - 33.13% net_rx_queue_update_kobjects ▒
> - kobject_put ▒
> - kobject_release ▒
> + 33.11% kobject_uevent ▒
> + 0.01% kobject_del ▒
> 0.00% rx_queue_release ▒
> - 33.08% device_del ▒
> + 32.75% kobject_uevent ▒
> + 0.17% device_remove_attrs ▒
> + 0.07% dpm_sysfs_remove ▒
> + 0.04% device_remove_class_symlinks ▒
> + 0.01% kobject_del ▒
> + 0.01% device_pm_remove ▒
> + 0.01% sysfs_remove_file_ns ▒
> + 0.00% klist_del ▒
> + 0.00% driver_deferred_probe_del ▒
> 0.00% cleanup_glue_dir.isra.14.part.15 ▒
> 0.00% to_acpi_device_node ▒
> 0.00% sysfs_remove_group ▒
> 0.00% klist_del ▒
> 0.00% device_remove_attrs ▒
> + 0.26% call_netdevice_notifiers_info ▒
> + 0.04% rtmsg_ifinfo_build_skb ▒
> + 0.01% rtmsg_ifinfo_send ▒
> 0.00% dev_uc_flush ▒
> 0.00% netif_reset_xps_queues_gt
>
> Someone can listen these uevents, so we can't stop sending them without
> breaking backward compatibility. We can try to optimize
> kobject_uevent...
Oh that is a surprise. We can definitely skip genenerating uevents for
network namespaces that are exiting because by definition no one can see
those network namespaces. If a socket existed that could see those
uevents it would hold a reference to the network namespace and as such
the network namespace could not exit.
That sounds like it is worth investigating a little more deeply.
I am surprised that allocation and freeing is so heavy we are spending
lots of time doing that. On the other hand kobj_bcast_filter is very
dumb and very late so I expect something can be moved earlier and make
that code cheaper with the tiniest bit of work.
Eric
^ permalink raw reply
* Re: bug in ixgbe_atr
From: Sowmini Varadhan @ 2016-10-14 3:48 UTC (permalink / raw)
To: Duyck, Alexander H; +Cc: netdev@vger.kernel.org
In-Reply-To: <B1C1DF2ACD01FD4881736AA51731BAB2A0DDE3@ORSMSX107.amr.corp.intel.com>
On (10/14/16 02:06), Duyck, Alexander H wrote:
> > + case ETH_P_IP:
> > + skb_header_pointer(skb, ETH_HLEN, sizeof (struct iphdr),
> > + &ip_hdr);
> > /* access ihl as u8 to avoid unaligned access on ia64 */
> > - hlen = (hdr.network[0] & 0x0F) << 2;
> > - l4_proto = hdr.ipv4->protocol;
> > + hlen = ip_hdr.ipv4.ihl << 2;
> > + l4_proto = ip_hdr.ipv4.protocol;
> > break;
:
> The problem is this will break other stuff, for example I have seen
> the ihl access actually cause problems with unaligned accesses as some
> architectures decide to pull it as a u32 and then mask it.
Yes, I noticed that u8 comment for ia64.. if that's the only issue
here, we could just reset hdr.network to &ip_hdr..
However, I suspect the above patch is probably not going to work for
the vlan case (it was just a first-pass hack)
> My advice would be to keep this simple. Add a check to make sure we
> have room for at least skb_headlen(skb) - 40 >= hrd.raw - skb->data.
I don't parse that- the hdr union in ixgbe_atr doesnt have a ->raw
field. Can you explain?
> Messing with the protocol bits will break stuff since there is support
> for tunneling also floating around in here now.
>
> I believe we are planning on dropping this code in favor of
> ndo_rx_flow_steer in the future. If we do that then the whole problem
> becomes moot.
Dropping it is fine with me I guess - maybe just return, if the
skb_headlen() doesnt have enough bytes for a network header,
i.e., skb_headlen is at least ETH_HLEN + sizeof (struct iphdr) for
ETH_P_IP, or ETH_HLEN + sizeof (struct ipv6hdr) for ETH_P_IPV6?
--Sowmini
^ permalink raw reply
* Re: Need help with mdiobus_register and phy
From: Andrew Lunn @ 2016-10-14 4:06 UTC (permalink / raw)
To: Timur Tabi; +Cc: netdev
In-Reply-To: <5800159B.3080708@codeaurora.org>
On Thu, Oct 13, 2016 at 06:15:39PM -0500, Timur Tabi wrote:
> I need help with a program I've discovered with my driver
> (ethernet/qualcomm/emac/) that occurs in the following process:
>
> 1. Load the driver
> 1a. Driver calls mdiobus_register (see emac_phy_config), which calls
> mdiobus_scan(), which calls get_phy_device()
> 1b. get_phy_device() performs MDIO bus operations, which means the
> bus and the external phy need to be active.
> 2. Bring up the interface
> 3. Bring down the interface. This calls phy_disconnect() which
> calls phy_detach() which calls phy_suspend(). This puts the
> external phy to sleep.
> 4. Unload the driver
> 5. Re-load the driver.
> 5a. Driver calls mdiobus_register again, but this time the call to
> get_phy_device() fails because the external phy is asleep and does
> not respond to MDIO transactions.
Hi Timur
Normally, a sleeping PHY does respond to MDIO. Otherwise, how do you
wake it?
So i assume this phy has some other means to wake it. What is this
means?
Andrew
^ permalink raw reply
* Re: [PATCH v4 net-next 0/4] act_mirred: Ingress actions support
From: Cong Wang @ 2016-10-14 4:24 UTC (permalink / raw)
To: Shmulik Ladkani
Cc: David Miller, Jamal Hadi Salim, Eric Dumazet, Daniel Borkmann,
Linux Kernel Network Developers
In-Reply-To: <1476338804-25440-1-git-send-email-shmulik.ladkani@gmail.com>
On Wed, Oct 12, 2016 at 11:06 PM, Shmulik Ladkani
<shmulik.ladkani@gmail.com> wrote:
> This patch series implements action mirred 'ingress' actions
> TCA_INGRESS_REDIR and TCA_INGRESS_MIRROR.
>
> This allows attaching filters whose target is to hand matching skbs into
> the rx processing of a specified device.
>
> v4:
> in 4/4, check ret code of netif_receive_skb, as suggested by Cong Wang
> v3:
> in 4/4, addressed non coherency due to reading m->tcfm_eaction multiple
> times, as spotted by Eric Dumazet
> v2:
> in 1/4, declare tcfm_mac_header_xmit as bool instead of int
>
For the whole series,
Acked-by: Cong Wang <xiyou.wangcong@gmail.com>
Thanks for your update!
^ permalink raw reply
* [PATCH v2] net: Require exact match for TCP socket lookups if dif is l3mdev
From: David Ahern @ 2016-10-14 4:47 UTC (permalink / raw)
To: netdev; +Cc: eric.dumazet, David Ahern
Currently, socket lookups for l3mdev (vrf) use cases can match a socket
that is bound to a port but not a device (ie., a global socket). If the
sysctl tcp_l3mdev_accept is not set this leads to ack packets going out
based on the main table even though the packet came in from an L3 domain.
The end result is that the connection does not establish creating
confusion for users since the service is running and a socket shows in
ss output. Fix by requiring an exact dif to sk_bound_dev_if match if the
skb came through an interface enslaved to an l3mdev device and the
tcp_l3mdev_accept is not set.
skb's through an l3mdev interface are marked by setting a flag in the
inet{6}_skb_parm. The IPv6 variant is already set. This patch adds the
flag for IPv4. Marking the skb avoids a device lookup on the dif.
The inet_skb_parm struct currently has a 1-byte hold following the
flags, so the flags is expanded to u16 without increasing the size of
the struct. This is needed to add another flag.
Fixes: 193125dbd8eb ("net: Introduce VRF device driver")
Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
---
v2
- reordered the checks in inet_exact_dif_match per Eric's comment
- changed the l3mdev determination from looking up the dif to using
a flag set on the skb which is much faster
drivers/net/vrf.c | 2 ++
include/linux/ipv6.h | 10 ++++++++++
include/net/ip.h | 13 ++++++++++++-
net/ipv4/inet_hashtables.c | 7 ++++---
net/ipv6/inet6_hashtables.c | 7 ++++---
5 files changed, 32 insertions(+), 7 deletions(-)
diff --git a/drivers/net/vrf.c b/drivers/net/vrf.c
index 85c271c70d42..820de6a9ddde 100644
--- a/drivers/net/vrf.c
+++ b/drivers/net/vrf.c
@@ -956,6 +956,7 @@ static struct sk_buff *vrf_ip6_rcv(struct net_device *vrf_dev,
if (skb->pkt_type == PACKET_LOOPBACK) {
skb->dev = vrf_dev;
skb->skb_iif = vrf_dev->ifindex;
+ IP6CB(skb)->flags |= IP6SKB_L3SLAVE;
skb->pkt_type = PACKET_HOST;
goto out;
}
@@ -996,6 +997,7 @@ static struct sk_buff *vrf_ip_rcv(struct net_device *vrf_dev,
{
skb->dev = vrf_dev;
skb->skb_iif = vrf_dev->ifindex;
+ IPCB(skb)->flags |= IPSKB_L3SLAVE;
/* loopback traffic; do not push through packet taps again.
* Reset pkt_type for upper layers to process skb
diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
index 7e9a789be5e0..dd3bb34aac8b 100644
--- a/include/linux/ipv6.h
+++ b/include/linux/ipv6.h
@@ -144,6 +144,16 @@ static inline int inet6_iif(const struct sk_buff *skb)
return l3_slave ? skb->skb_iif : IP6CB(skb)->iif;
}
+static inline bool inet6_exact_dif_match(struct net *net, struct sk_buff *skb)
+{
+#ifdef CONFIG_NET_L3_MASTER_DEV
+ if (!net->ipv4.sysctl_tcp_l3mdev_accept &&
+ IP6CB(skb)->flags & IP6SKB_L3SLAVE)
+ return true;
+#endif
+ return false;
+}
+
struct tcp6_request_sock {
struct tcp_request_sock tcp6rsk_tcp;
};
diff --git a/include/net/ip.h b/include/net/ip.h
index bc43c0fcae12..adeb9a2c9c16 100644
--- a/include/net/ip.h
+++ b/include/net/ip.h
@@ -38,7 +38,7 @@ struct sock;
struct inet_skb_parm {
int iif;
struct ip_options opt; /* Compiled IP options */
- unsigned char flags;
+ __u16 flags;
#define IPSKB_FORWARDED BIT(0)
#define IPSKB_XFRM_TUNNEL_SIZE BIT(1)
@@ -48,6 +48,7 @@ struct inet_skb_parm {
#define IPSKB_DOREDIRECT BIT(5)
#define IPSKB_FRAG_PMTU BIT(6)
#define IPSKB_FRAG_SEGS BIT(7)
+#define IPSKB_L3SLAVE BIT(8)
u16 frag_max_size;
};
@@ -71,6 +72,16 @@ struct ipcm_cookie {
#define IPCB(skb) ((struct inet_skb_parm*)((skb)->cb))
#define PKTINFO_SKB_CB(skb) ((struct in_pktinfo *)((skb)->cb))
+static inline bool inet_exact_dif_match(struct net *net, struct sk_buff *skb)
+{
+#ifdef CONFIG_NET_L3_MASTER_DEV
+ if (!net->ipv4.sysctl_tcp_l3mdev_accept &&
+ IPCB(skb)->flags & IPSKB_L3SLAVE)
+ return true;
+#endif
+ return false;
+}
+
struct ip_ra_chain {
struct ip_ra_chain __rcu *next;
struct sock *sk;
diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
index 77c20a489218..0d6b996fd801 100644
--- a/net/ipv4/inet_hashtables.c
+++ b/net/ipv4/inet_hashtables.c
@@ -172,7 +172,7 @@ EXPORT_SYMBOL_GPL(__inet_inherit_port);
static inline int compute_score(struct sock *sk, struct net *net,
const unsigned short hnum, const __be32 daddr,
- const int dif)
+ const int dif, bool exact_dif)
{
int score = -1;
struct inet_sock *inet = inet_sk(sk);
@@ -186,7 +186,7 @@ static inline int compute_score(struct sock *sk, struct net *net,
return -1;
score += 4;
}
- if (sk->sk_bound_dev_if) {
+ if (sk->sk_bound_dev_if || exact_dif) {
if (sk->sk_bound_dev_if != dif)
return -1;
score += 4;
@@ -215,11 +215,12 @@ struct sock *__inet_lookup_listener(struct net *net,
unsigned int hash = inet_lhashfn(net, hnum);
struct inet_listen_hashbucket *ilb = &hashinfo->listening_hash[hash];
int score, hiscore = 0, matches = 0, reuseport = 0;
+ bool exact_dif = inet_exact_dif_match(net, skb);
struct sock *sk, *result = NULL;
u32 phash = 0;
sk_for_each_rcu(sk, &ilb->head) {
- score = compute_score(sk, net, hnum, daddr, dif);
+ score = compute_score(sk, net, hnum, daddr, dif, exact_dif);
if (score > hiscore) {
reuseport = sk->sk_reuseport;
if (reuseport) {
diff --git a/net/ipv6/inet6_hashtables.c b/net/ipv6/inet6_hashtables.c
index 00cf28ad4565..2fd0374a35b1 100644
--- a/net/ipv6/inet6_hashtables.c
+++ b/net/ipv6/inet6_hashtables.c
@@ -96,7 +96,7 @@ EXPORT_SYMBOL(__inet6_lookup_established);
static inline int compute_score(struct sock *sk, struct net *net,
const unsigned short hnum,
const struct in6_addr *daddr,
- const int dif)
+ const int dif, bool exact_dif)
{
int score = -1;
@@ -109,7 +109,7 @@ static inline int compute_score(struct sock *sk, struct net *net,
return -1;
score++;
}
- if (sk->sk_bound_dev_if) {
+ if (sk->sk_bound_dev_if || exact_dif) {
if (sk->sk_bound_dev_if != dif)
return -1;
score++;
@@ -131,11 +131,12 @@ struct sock *inet6_lookup_listener(struct net *net,
unsigned int hash = inet_lhashfn(net, hnum);
struct inet_listen_hashbucket *ilb = &hashinfo->listening_hash[hash];
int score, hiscore = 0, matches = 0, reuseport = 0;
+ bool exact_dif = inet6_exact_dif_match(net, skb);
struct sock *sk, *result = NULL;
u32 phash = 0;
sk_for_each(sk, &ilb->head) {
- score = compute_score(sk, net, hnum, daddr, dif);
+ score = compute_score(sk, net, hnum, daddr, dif, exact_dif);
if (score > hiscore) {
reuseport = sk->sk_reuseport;
if (reuseport) {
--
2.1.4
^ permalink raw reply related
* Re: [patch net-next RFC 0/6] Add support for offloading packet-sampling
From: Roopa Prabhu @ 2016-10-14 5:02 UTC (permalink / raw)
To: Jiri Pirko
Cc: Jamal Hadi Salim, netdev@vger.kernel.org, davem@davemloft.net,
Yotam Gigi, Ido Schimmel, Elad Raz, Nogah Frankel, Or Gerlitz,
geert+renesas, stephen@networkplumber.org, Cong Wang,
Guenter Roeck, Shrijeet Mukherjee
In-Reply-To: <20161013124526.GF1816@nanopsycho.orion>
On Thu, Oct 13, 2016 at 5:45 AM, Jiri Pirko <jiri@resnulli.us> wrote:
> Thu, Oct 13, 2016 at 02:30:19PM CEST, jhs@mojatatu.com wrote:
>>On 16-10-13 08:10 AM, Jiri Pirko wrote:
>>> Thu, Oct 13, 2016 at 01:49:07PM CEST, jhs@mojatatu.com wrote:
>>> > On 16-10-13 04:48 AM, Jiri Pirko wrote:
>>
>>[..]
>>> > Roopa, did you mean eth1 as the new device or did you mean just in
>>> > general config requiring a device to be specified or did you mean a new
>>> > cpu netdev being needed? I couldnt tell from the patch.
>>>
>>> You just have to have some netdev to use to funnel the IFE headered
>>> sample skbs to userspace. A dummy or a tap.
>>>
>>
>>I see.
>>So with nflog you get basically a backend using a netlink socket
>>but in your case you will redirect to tuntap for the case of local
>>sflow but some other device for remote? I am assuming using dummy
>>would require a packet socket as means of retrieving the data.
>
> Correct. The idea is that the userspace app would create a tap device,
> setup the sampling packets to be sent there and recieve them
> over chardev. Or the remote delivery could be use to push the sampling
> packet to a remote host.
>
>
>>If you take the structuring of the metadata that nflog uses it should
>>be easy to transpose.
>
> Yes, we do it with IFE, this patchset implements that.
>
>
>>To Roopa's point, however: Would it not make sense to support nflog
>>(in addition?).
>>
[sorry responding to all conversations so far here]
using ife for delivery of sampled packets to remote is a good option
to have if you have users.
so far I have seen agents collecting samples locally and have their
own protocol to ship them
to a collector (example sflow). Just bringing that up so that we don't
optimize for the less common case
and make the common case difficult to use :).
In my conversations with the sflow people (founders) and others,
netlink as a mechanism for sampled packet
delivery (similar to ulog/nflog) has proven useful and they see it as
a great API to standardize on going forward (Given they are
already using netlink for collecting other samples like stats etc).
something to thing about.
The people I know collecting samples are happy with having netfilter.
agreed that tc already has an existing hw
offload mechanism. and I was not suggesting giving up on tc either.
and also to jiri, agree, I don't think logging from the driver is a
good option. I was merely suggesting
having a similar option without the need for a new collector device.
The three steps in the patch series to collect samples + a device
seems a bit heavy weight.
but, if you think you have users for it, sure. having multiple api's
is also an option.
But api's come with a cost of maintaining them for ever.
^ permalink raw reply
* Re: [PATCH net-next 1/2] lwtunnel: Add destroy state operation
From: Roopa Prabhu @ 2016-10-14 5:58 UTC (permalink / raw)
To: Tom Herbert; +Cc: davem, netdev, kernel-team
In-Reply-To: <20161014005743.288956-2-tom@herbertland.com>
On 10/13/16, 5:57 PM, Tom Herbert wrote:
> Users of lwt tunnels may set up some secondary state in build_state
> function. Add a corresponding destroy_state function to allow users to
> clean up state. This destroy state function is called from lwstate_free.
> Also, we now free lwstate using kfree_rcu so user can assume structure
> is not freed before rcu.
>
> Signed-off-by: Tom Herbert <tom@herbertland.com>
Acked-by: Roopa Prabhu <roopa@cumulusnetworks.com>
this will be useful elsewhere too, thanks!.
^ permalink raw reply
* Re: [PATCH net 4/5] net/ncsi: Choose hot channel as active one if necessary
From: Joel Stanley @ 2016-10-14 6:02 UTC (permalink / raw)
To: Gavin Shan; +Cc: netdev, davem
In-Reply-To: <1476413614-24586-5-git-send-email-gwshan@linux.vnet.ibm.com>
On Fri, Oct 14, 2016 at 1:23 PM, Gavin Shan <gwshan@linux.vnet.ibm.com> wrote:
> The issue was found on BCM5718 which has two NCSI channels in one
> package: C0 and C1. C0 is in link-up state while C1 is in link-down
> state. C0 is chosen as active channel until unplugging and plugging
> C0's cable: On unplugging C0's cable, LSC (Link State Change) AEN
> packet received on C0 to report link-down event. After that, C1 is
> chosen as active channel. LSC AEN for link-up event is lost on C0
> when plugging C0's cable back. We lose the network even C0 is usable.
Why do we lose the LCS AEN packet?
Is this a bug in the BCM5718? If so, we shouldn't put it in the common
ncsi code without adding a quirk for that hardware.
>
> This resolves the issue by recording the (hot) channel that was ever
> chosen as active one. The hot channel is chosen to be active one
> if none of available channels in link-up state. With this, C0 is still
> the active one after unplugging C0's cable. LSC AEN packet received
> on C0 when plugging its cable back.
>
> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
> ---
> net/ncsi/internal.h | 1 +
> net/ncsi/ncsi-manage.c | 22 +++++++++++++++++++---
> 2 files changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
> index eac4858..1308a56 100644
> --- a/net/ncsi/internal.h
> +++ b/net/ncsi/internal.h
> @@ -265,6 +265,7 @@ struct ncsi_dev_priv {
> #endif
> unsigned int package_num; /* Number of packages */
> struct list_head packages; /* List of packages */
> + struct ncsi_channel *hot_channel; /* Channel was ever active */
> struct ncsi_request requests[256]; /* Request table */
> unsigned int request_id; /* Last used request ID */
> #define NCSI_REQ_START_IDX 1
> diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
> index e959979..cccedcf 100644
> --- a/net/ncsi/ncsi-manage.c
> +++ b/net/ncsi/ncsi-manage.c
> @@ -625,6 +625,7 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
> struct net_device *dev = nd->dev;
> struct ncsi_package *np = ndp->active_package;
> struct ncsi_channel *nc = ndp->active_channel;
> + struct ncsi_channel *hot_nc = NULL;
> struct ncsi_cmd_arg nca;
> unsigned char index;
> unsigned long flags;
> @@ -730,12 +731,20 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
> break;
> case ncsi_dev_state_config_done:
> spin_lock_irqsave(&nc->lock, flags);
> - if (nc->modes[NCSI_MODE_LINK].data[2] & 0x1)
> + if (nc->modes[NCSI_MODE_LINK].data[2] & 0x1) {
> + hot_nc = nc;
> nc->state = NCSI_CHANNEL_ACTIVE;
> - else
> + } else {
> + hot_nc = NULL;
> nc->state = NCSI_CHANNEL_INACTIVE;
> + }
> spin_unlock_irqrestore(&nc->lock, flags);
>
> + /* Update the hot channel */
> + spin_lock_irqsave(&ndp->lock, flags);
> + ndp->hot_channel = hot_nc;
> + spin_unlock_irqrestore(&ndp->lock, flags);
> +
> ncsi_start_channel_monitor(nc);
> ncsi_process_next_channel(ndp);
> break;
> @@ -753,10 +762,14 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
> static int ncsi_choose_active_channel(struct ncsi_dev_priv *ndp)
> {
> struct ncsi_package *np;
> - struct ncsi_channel *nc, *found;
> + struct ncsi_channel *nc, *found, *hot_nc;
> struct ncsi_channel_mode *ncm;
> unsigned long flags;
>
> + spin_lock_irqsave(&ndp->lock, flags);
> + hot_nc = ndp->hot_channel;
> + spin_unlock_irqrestore(&ndp->lock, flags);
> +
> /* The search is done once an inactive channel with up
> * link is found.
> */
> @@ -774,6 +787,9 @@ static int ncsi_choose_active_channel(struct ncsi_dev_priv *ndp)
> if (!found)
> found = nc;
>
> + if (nc == hot_nc)
> + found = nc;
> +
> ncm = &nc->modes[NCSI_MODE_LINK];
> if (ncm->data[2] & 0x1) {
> spin_unlock_irqrestore(&nc->lock, flags);
> --
> 2.1.0
>
^ permalink raw reply
* Re: [PATCH net 3/5] net/ncsi: Fix stale link state of inactive channels on failover
From: Joel Stanley @ 2016-10-14 6:02 UTC (permalink / raw)
To: Gavin Shan; +Cc: netdev, davem
In-Reply-To: <1476413614-24586-4-git-send-email-gwshan@linux.vnet.ibm.com>
On Fri, Oct 14, 2016 at 1:23 PM, Gavin Shan <gwshan@linux.vnet.ibm.com> wrote:
> The issue was found on BCM5718 which has two NCSI channels in one
> package: C0 and C1. Both of them are connected to different LANs,
> means they are in link-up state and C0 is chosen as the active
> one until resetting BCM5718 happens as below.
>
> Resetting BCM5718 results in LSC (Link State Change) AEN packet
> received on C0, meaning LSC AEN is missed on C1. When LSC AEN packet
> received on C0 to report link-down, it fails over to C1 because C1
> is in link-up state as software can see. However, C1 is in link-down
> state in hardware. It means the link state is out of synchronization
> between hardware and software, resulting in inappropriate channel (C1)
> selected as active one.
>
> This resolves the issue by sending separate GLS (Get Link Status)
> commands to all channels in the package before trying to do failover.
> The last link state on all channels in the package is retrieved. With
> it, C0 is selected as active one as expected.
I follow this, and can see that happening in the
ncsi_dev_state_suspend_gls state. However, what is
> - nd->state = ncsi_dev_state_suspend_dcnt;
> + if (ndp->flags & NCSI_DEV_RESHUFFLE)
> + nd->state = ncsi_dev_state_suspend_gls;
> + else
> + nd->state = ncsi_dev_state_suspend_dcnt;
However, what is this doing? I'm not quite sure what
NCSI_DEV_RESHUFFLE is and why we enable it?
>
> ret = ncsi_xmit_cmd(&nca);
> if (ret)
> goto error;
>
> break;
> + case ncsi_dev_state_suspend_gls:
> + ndp->pending_req_num = np->channel_num;
> +
> + nca.type = NCSI_PKT_CMD_GLS;
> + nca.package = np->id;
> + nd->state = ncsi_dev_state_suspend_dcnt;
> +
> + NCSI_FOR_EACH_CHANNEL(np, nc) {
> + nca.channel = nc->id;
> + ret = ncsi_xmit_cmd(&nca);
> + if (ret)
> + goto error;
> + }
> +
> + break;
> case ncsi_dev_state_suspend_dcnt:
> case ncsi_dev_state_suspend_dc:
> case ncsi_dev_state_suspend_deselect:
> --
> 2.1.0
>
^ 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