* Re: [PATCH net-next] switchdev: Remove unused transaction item queue
From: Jiri Pirko @ 2019-02-28 7:02 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev, David S. Miller, Ido Schimmel, open list,
open list:STAGING SUBSYSTEM, moderated list:ETHERNET BRIDGE, jiri,
andrew, vivien.didelot
In-Reply-To: <20190228002916.24904-1-f.fainelli@gmail.com>
Thu, Feb 28, 2019 at 01:29:16AM CET, f.fainelli@gmail.com wrote:
>There are no more in tree users of the
>switchdev_trans_item_{dequeue,enqueue} or switchdev_trans_item structure
>in the kernel since commit 00fc0c51e35b ("rocker: Change world_ops API
>and implementation to be switchdev independant").
>
>Remove this unused code and update the documentation accordingly since.
>
>Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
^ permalink raw reply
* Re: [PATCH v3] xfrm: policy: Fix possible user after free in __xfrm_policy_unlink
From: YueHaibing @ 2019-02-28 7:14 UTC (permalink / raw)
To: steffen.klassert, herbert, davem; +Cc: linux-kernel, netdev
In-Reply-To: <20190228065623.21628-1-yuehaibing@huawei.com>
Pls ignore this, will fix the patch title
On 2019/2/28 14:56, Yue Haibing wrote:
> From: YueHaibing <yuehaibing@huawei.com>
>
> UBSAN report this:
>
> UBSAN: Undefined behaviour in net/xfrm/xfrm_policy.c:1289:24
> index 6 is out of range for type 'unsigned int [6]'
> CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.4.162-514.55.6.9.x86_64+ #13
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1ubuntu1 04/01/2014
> 0000000000000000 1466cf39b41b23c9 ffff8801f6b07a58 ffffffff81cb35f4
> 0000000041b58ab3 ffffffff83230f9c ffffffff81cb34e0 ffff8801f6b07a80
> ffff8801f6b07a20 1466cf39b41b23c9 ffffffff851706e0 ffff8801f6b07ae8
> Call Trace:
> <IRQ> [<ffffffff81cb35f4>] __dump_stack lib/dump_stack.c:15 [inline]
> <IRQ> [<ffffffff81cb35f4>] dump_stack+0x114/0x1a0 lib/dump_stack.c:51
> [<ffffffff81d94225>] ubsan_epilogue+0x12/0x8f lib/ubsan.c:164
> [<ffffffff81d954db>] __ubsan_handle_out_of_bounds+0x16e/0x1b2 lib/ubsan.c:382
> [<ffffffff82a25acd>] __xfrm_policy_unlink+0x3dd/0x5b0 net/xfrm/xfrm_policy.c:1289
> [<ffffffff82a2e572>] xfrm_policy_delete+0x52/0xb0 net/xfrm/xfrm_policy.c:1309
> [<ffffffff82a3319b>] xfrm_policy_timer+0x30b/0x590 net/xfrm/xfrm_policy.c:243
> [<ffffffff813d3927>] call_timer_fn+0x237/0x990 kernel/time/timer.c:1144
> [<ffffffff813d8e7e>] __run_timers kernel/time/timer.c:1218 [inline]
> [<ffffffff813d8e7e>] run_timer_softirq+0x6ce/0xb80 kernel/time/timer.c:1401
> [<ffffffff8120d6f9>] __do_softirq+0x299/0xe10 kernel/softirq.c:273
> [<ffffffff8120e676>] invoke_softirq kernel/softirq.c:350 [inline]
> [<ffffffff8120e676>] irq_exit+0x216/0x2c0 kernel/softirq.c:391
> [<ffffffff82c5edab>] exiting_irq arch/x86/include/asm/apic.h:652 [inline]
> [<ffffffff82c5edab>] smp_apic_timer_interrupt+0x8b/0xc0 arch/x86/kernel/apic/apic.c:926
> [<ffffffff82c5c985>] apic_timer_interrupt+0xa5/0xb0 arch/x86/entry/entry_64.S:735
> <EOI> [<ffffffff81188096>] ? native_safe_halt+0x6/0x10 arch/x86/include/asm/irqflags.h:52
> [<ffffffff810834d7>] arch_safe_halt arch/x86/include/asm/paravirt.h:111 [inline]
> [<ffffffff810834d7>] default_idle+0x27/0x430 arch/x86/kernel/process.c:446
> [<ffffffff81085f05>] arch_cpu_idle+0x15/0x20 arch/x86/kernel/process.c:437
> [<ffffffff8132abc3>] default_idle_call+0x53/0x90 kernel/sched/idle.c:92
> [<ffffffff8132b32d>] cpuidle_idle_call kernel/sched/idle.c:156 [inline]
> [<ffffffff8132b32d>] cpu_idle_loop kernel/sched/idle.c:251 [inline]
> [<ffffffff8132b32d>] cpu_startup_entry+0x60d/0x9a0 kernel/sched/idle.c:299
> [<ffffffff8113e119>] start_secondary+0x3c9/0x560 arch/x86/kernel/smpboot.c:245
>
> The issue is triggered as this:
>
> xfrm_add_policy
> -->verify_newpolicy_info //check the index provided by user with XFRM_POLICY_MAX
> //In my case, the index is 0x6E6BB6, so it pass the check.
> -->xfrm_policy_construct //copy the user's policy and set xfrm_policy_timer
> -->xfrm_policy_insert
> --> __xfrm_policy_link //use the orgin dir, in my case is 2
> --> xfrm_gen_index //generate policy index, there is 0x6E6BB6
>
> then xfrm_policy_timer be fired
>
> xfrm_policy_timer
> --> xfrm_policy_id2dir //get dir from (policy index & 7), in my case is 6
> --> xfrm_policy_delete
> --> __xfrm_policy_unlink //access policy_count[dir], trigger out of range access
>
> Add xfrm_policy_id2dir check in verify_newpolicy_info, make sure the computed dir is
> valid, to fix the issue.
>
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Fixes: e682adf021be ("xfrm: Try to honor policy index if it's supplied by user")
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> ---
> v3: fold the check
> v2: fix index check in verify_newpolicy_info
> ---
> net/xfrm/xfrm_user.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
> index a131f9f..8d4d52f 100644
> --- a/net/xfrm/xfrm_user.c
> +++ b/net/xfrm/xfrm_user.c
> @@ -1424,7 +1424,7 @@ static int verify_newpolicy_info(struct xfrm_userpolicy_info *p)
> ret = verify_policy_dir(p->dir);
> if (ret)
> return ret;
> - if (p->index && ((p->index & XFRM_POLICY_MAX) != p->dir))
> + if (p->index && (xfrm_policy_id2dir(p->index) != p->dir))
> return -EINVAL;
>
> return 0;
>
^ permalink raw reply
* [PATCH] xfrm: policy: Fix out-of-bound array accesses in __xfrm_policy_unlink
From: Yue Haibing @ 2019-02-28 7:18 UTC (permalink / raw)
To: steffen.klassert, herbert, davem; +Cc: linux-kernel, netdev, YueHaibing
From: YueHaibing <yuehaibing@huawei.com>
UBSAN report this:
UBSAN: Undefined behaviour in net/xfrm/xfrm_policy.c:1289:24
index 6 is out of range for type 'unsigned int [6]'
CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.4.162-514.55.6.9.x86_64+ #13
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1ubuntu1 04/01/2014
0000000000000000 1466cf39b41b23c9 ffff8801f6b07a58 ffffffff81cb35f4
0000000041b58ab3 ffffffff83230f9c ffffffff81cb34e0 ffff8801f6b07a80
ffff8801f6b07a20 1466cf39b41b23c9 ffffffff851706e0 ffff8801f6b07ae8
Call Trace:
<IRQ> [<ffffffff81cb35f4>] __dump_stack lib/dump_stack.c:15 [inline]
<IRQ> [<ffffffff81cb35f4>] dump_stack+0x114/0x1a0 lib/dump_stack.c:51
[<ffffffff81d94225>] ubsan_epilogue+0x12/0x8f lib/ubsan.c:164
[<ffffffff81d954db>] __ubsan_handle_out_of_bounds+0x16e/0x1b2 lib/ubsan.c:382
[<ffffffff82a25acd>] __xfrm_policy_unlink+0x3dd/0x5b0 net/xfrm/xfrm_policy.c:1289
[<ffffffff82a2e572>] xfrm_policy_delete+0x52/0xb0 net/xfrm/xfrm_policy.c:1309
[<ffffffff82a3319b>] xfrm_policy_timer+0x30b/0x590 net/xfrm/xfrm_policy.c:243
[<ffffffff813d3927>] call_timer_fn+0x237/0x990 kernel/time/timer.c:1144
[<ffffffff813d8e7e>] __run_timers kernel/time/timer.c:1218 [inline]
[<ffffffff813d8e7e>] run_timer_softirq+0x6ce/0xb80 kernel/time/timer.c:1401
[<ffffffff8120d6f9>] __do_softirq+0x299/0xe10 kernel/softirq.c:273
[<ffffffff8120e676>] invoke_softirq kernel/softirq.c:350 [inline]
[<ffffffff8120e676>] irq_exit+0x216/0x2c0 kernel/softirq.c:391
[<ffffffff82c5edab>] exiting_irq arch/x86/include/asm/apic.h:652 [inline]
[<ffffffff82c5edab>] smp_apic_timer_interrupt+0x8b/0xc0 arch/x86/kernel/apic/apic.c:926
[<ffffffff82c5c985>] apic_timer_interrupt+0xa5/0xb0 arch/x86/entry/entry_64.S:735
<EOI> [<ffffffff81188096>] ? native_safe_halt+0x6/0x10 arch/x86/include/asm/irqflags.h:52
[<ffffffff810834d7>] arch_safe_halt arch/x86/include/asm/paravirt.h:111 [inline]
[<ffffffff810834d7>] default_idle+0x27/0x430 arch/x86/kernel/process.c:446
[<ffffffff81085f05>] arch_cpu_idle+0x15/0x20 arch/x86/kernel/process.c:437
[<ffffffff8132abc3>] default_idle_call+0x53/0x90 kernel/sched/idle.c:92
[<ffffffff8132b32d>] cpuidle_idle_call kernel/sched/idle.c:156 [inline]
[<ffffffff8132b32d>] cpu_idle_loop kernel/sched/idle.c:251 [inline]
[<ffffffff8132b32d>] cpu_startup_entry+0x60d/0x9a0 kernel/sched/idle.c:299
[<ffffffff8113e119>] start_secondary+0x3c9/0x560 arch/x86/kernel/smpboot.c:245
The issue is triggered as this:
xfrm_add_policy
-->verify_newpolicy_info //check the index provided by user with XFRM_POLICY_MAX
//In my case, the index is 0x6E6BB6, so it pass the check.
-->xfrm_policy_construct //copy the user's policy and set xfrm_policy_timer
-->xfrm_policy_insert
--> __xfrm_policy_link //use the orgin dir, in my case is 2
--> xfrm_gen_index //generate policy index, there is 0x6E6BB6
then xfrm_policy_timer be fired
xfrm_policy_timer
--> xfrm_policy_id2dir //get dir from (policy index & 7), in my case is 6
--> xfrm_policy_delete
--> __xfrm_policy_unlink //access policy_count[dir], trigger out of range access
Add xfrm_policy_id2dir check in verify_newpolicy_info, make sure the computed dir is
valid, to fix the issue.
Reported-by: Hulk Robot <hulkci@huawei.com>
Fixes: e682adf021be ("xfrm: Try to honor policy index if it's supplied by user")
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
net/xfrm/xfrm_user.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index a131f9f..8d4d52f 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -1424,7 +1424,7 @@ static int verify_newpolicy_info(struct xfrm_userpolicy_info *p)
ret = verify_policy_dir(p->dir);
if (ret)
return ret;
- if (p->index && ((p->index & XFRM_POLICY_MAX) != p->dir))
+ if (p->index && (xfrm_policy_id2dir(p->index) != p->dir))
return -EINVAL;
return 0;
--
2.7.0
^ permalink raw reply related
* [PATCH] net: mscc: Enable all ports in QSGMII
From: Kavyasree.Kotagiri @ 2019-02-28 7:32 UTC (permalink / raw)
To: alexandre.belloni, davem
Cc: UNGLinuxDriver, Chakri.Ponnuri, netdev, Kavyasree.Kotagiri,
Steen.Hegelund
From: Kavya Sree Kotagiri <kavyasree.kotagiri@microchip.com>
When Ocelot phy-mode is QSGMII, all 4 ports involved in
QSGMII shall be kept out of reset and
Tx lanes shall be enabled to pass the data.
Fixes: a556c76adc05 ("net: mscc: Add initial Ocelot switch support")
Signed-off-by: Kavya Sree Kotagiri <kavyasree.kotagiri@microchip.com>
Signed-off-by: Steen Hegelund <Steen.Hegelund@microchip.com>
Co-developed-by: Steen Hegelund <Steen.Hegelund@microchip.com>
---
drivers/net/ethernet/mscc/ocelot_board.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/mscc/ocelot_board.c b/drivers/net/ethernet/mscc/ocelot_board.c
index ca3ea2f..80d8779 100644
--- a/drivers/net/ethernet/mscc/ocelot_board.c
+++ b/drivers/net/ethernet/mscc/ocelot_board.c
@@ -267,6 +267,7 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
struct phy *serdes;
void __iomem *regs;
char res_name[8];
+ int phy_mode;
u32 port;
if (of_property_read_u32(portnp, "reg", &port))
@@ -292,11 +293,11 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
if (err)
return err;
- err = of_get_phy_mode(portnp);
- if (err < 0)
+ phy_mode = of_get_phy_mode(portnp);
+ if (phy_mode < 0)
ocelot->ports[port]->phy_mode = PHY_INTERFACE_MODE_NA;
else
- ocelot->ports[port]->phy_mode = err;
+ ocelot->ports[port]->phy_mode = phy_mode;
switch (ocelot->ports[port]->phy_mode) {
case PHY_INTERFACE_MODE_NA:
@@ -304,6 +305,13 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
case PHY_INTERFACE_MODE_SGMII:
break;
case PHY_INTERFACE_MODE_QSGMII:
+ /* Ensure clock signals and speed is set on all
+ * QSGMII links
+ */
+ ocelot_port_writel(ocelot->ports[port],
+ DEV_CLOCK_CFG_LINK_SPEED
+ (OCELOT_SPEED_1000),
+ DEV_CLOCK_CFG);
break;
default:
dev_err(ocelot->dev,
--
1.9.1
^ permalink raw reply related
* Re: [PATCH RESEND net] net: phy: xgmiitorgmii: Support generic PHY status read
From: Harini Katakam @ 2019-02-28 7:33 UTC (permalink / raw)
To: Michal Simek
Cc: Paul Kocialkowski, Andrew Lunn, Florian Fainelli, netdev,
linux-arm-kernel, linux-kernel, David S . Miller,
Thomas Petazzoni, Heiner Kallweit
In-Reply-To: <CAFcVECJPtc3VQjK4WLCz1Wc4nopXnsLTKXFFY1ZpecgC+_PhyA@mail.gmail.com>
Hi,
On Wed, Feb 27, 2019 at 2:35 PM Harini Katakam <harinik@xilinx.com> wrote:
>
> Hi Andrew, Paul,
>
> On Wed, Feb 27, 2019 at 2:15 PM Michal Simek <michal.simek@xilinx.com> wrote:
> >
> > On 21. 02. 19 12:03, Michal Simek wrote:
> > > On 21. 02. 19 11:24, Paul Kocialkowski wrote:
> > >> Hi,
> > >>
> > >> On Wed, 2019-02-20 at 07:58 +0100, Michal Simek wrote:
> > >>> Hi,
> > >>>
> > >>> On 19. 02. 19 18:25, Andrew Lunn wrote:
<snip>
> > >> Understood. I think we need to start a discussion about how the general
> > >> design of this driver can be improved.
> > >>
> > >> In particular, I wonder if it could work better to make this driver a
> > >> PHY driver that just redirects all its ops to the actual PHY driver,
> > >> except for read_status where it should also add some code.
>
> Thanks, I'm looking into this option and also a way to expose the correct
> interface mode setting as you mentioned below. I'll get back before the
> end of the week. Please do let me know if you have any further suggestions.
>
This IP does not have a PHY identifier or status register that can be accessed
from the phy framework. We could discuss with our design team to add these
in the future. But that would take sometime and this version should be still be
supported. Also, if this IP has a PHY driver, then two phy drivers would have
to be probed which are connected in a serial manner and I believe I'll have to
update the framework to support that. Could you please let me know if you have
any inputs on this?
OR since this is just a bridge IP, is it acceptable to address the error cases?
-> Module loading/unloading
-> Spinlocks for protection
-> Correct phy mode information to the driver.
-> Any other concerns
I could do a respin of this patch after addressing Andrew's comments:
https://patchwork.kernel.org/patch/9290231/
Regards,
Harini
^ permalink raw reply
* Re: [PATCH] xfrm: policy: Fix out-of-bound array accesses in __xfrm_policy_unlink
From: Herbert Xu @ 2019-02-28 7:52 UTC (permalink / raw)
To: Yue Haibing; +Cc: steffen.klassert, davem, linux-kernel, netdev
In-Reply-To: <20190228071859.17768-1-yuehaibing@huawei.com>
On Thu, Feb 28, 2019 at 03:18:59PM +0800, Yue Haibing wrote:
> From: YueHaibing <yuehaibing@huawei.com>
>
> UBSAN report this:
>
> UBSAN: Undefined behaviour in net/xfrm/xfrm_policy.c:1289:24
> index 6 is out of range for type 'unsigned int [6]'
> CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.4.162-514.55.6.9.x86_64+ #13
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1ubuntu1 04/01/2014
> 0000000000000000 1466cf39b41b23c9 ffff8801f6b07a58 ffffffff81cb35f4
> 0000000041b58ab3 ffffffff83230f9c ffffffff81cb34e0 ffff8801f6b07a80
> ffff8801f6b07a20 1466cf39b41b23c9 ffffffff851706e0 ffff8801f6b07ae8
> Call Trace:
> <IRQ> [<ffffffff81cb35f4>] __dump_stack lib/dump_stack.c:15 [inline]
> <IRQ> [<ffffffff81cb35f4>] dump_stack+0x114/0x1a0 lib/dump_stack.c:51
> [<ffffffff81d94225>] ubsan_epilogue+0x12/0x8f lib/ubsan.c:164
> [<ffffffff81d954db>] __ubsan_handle_out_of_bounds+0x16e/0x1b2 lib/ubsan.c:382
> [<ffffffff82a25acd>] __xfrm_policy_unlink+0x3dd/0x5b0 net/xfrm/xfrm_policy.c:1289
> [<ffffffff82a2e572>] xfrm_policy_delete+0x52/0xb0 net/xfrm/xfrm_policy.c:1309
> [<ffffffff82a3319b>] xfrm_policy_timer+0x30b/0x590 net/xfrm/xfrm_policy.c:243
> [<ffffffff813d3927>] call_timer_fn+0x237/0x990 kernel/time/timer.c:1144
> [<ffffffff813d8e7e>] __run_timers kernel/time/timer.c:1218 [inline]
> [<ffffffff813d8e7e>] run_timer_softirq+0x6ce/0xb80 kernel/time/timer.c:1401
> [<ffffffff8120d6f9>] __do_softirq+0x299/0xe10 kernel/softirq.c:273
> [<ffffffff8120e676>] invoke_softirq kernel/softirq.c:350 [inline]
> [<ffffffff8120e676>] irq_exit+0x216/0x2c0 kernel/softirq.c:391
> [<ffffffff82c5edab>] exiting_irq arch/x86/include/asm/apic.h:652 [inline]
> [<ffffffff82c5edab>] smp_apic_timer_interrupt+0x8b/0xc0 arch/x86/kernel/apic/apic.c:926
> [<ffffffff82c5c985>] apic_timer_interrupt+0xa5/0xb0 arch/x86/entry/entry_64.S:735
> <EOI> [<ffffffff81188096>] ? native_safe_halt+0x6/0x10 arch/x86/include/asm/irqflags.h:52
> [<ffffffff810834d7>] arch_safe_halt arch/x86/include/asm/paravirt.h:111 [inline]
> [<ffffffff810834d7>] default_idle+0x27/0x430 arch/x86/kernel/process.c:446
> [<ffffffff81085f05>] arch_cpu_idle+0x15/0x20 arch/x86/kernel/process.c:437
> [<ffffffff8132abc3>] default_idle_call+0x53/0x90 kernel/sched/idle.c:92
> [<ffffffff8132b32d>] cpuidle_idle_call kernel/sched/idle.c:156 [inline]
> [<ffffffff8132b32d>] cpu_idle_loop kernel/sched/idle.c:251 [inline]
> [<ffffffff8132b32d>] cpu_startup_entry+0x60d/0x9a0 kernel/sched/idle.c:299
> [<ffffffff8113e119>] start_secondary+0x3c9/0x560 arch/x86/kernel/smpboot.c:245
>
> The issue is triggered as this:
>
> xfrm_add_policy
> -->verify_newpolicy_info //check the index provided by user with XFRM_POLICY_MAX
> //In my case, the index is 0x6E6BB6, so it pass the check.
> -->xfrm_policy_construct //copy the user's policy and set xfrm_policy_timer
> -->xfrm_policy_insert
> --> __xfrm_policy_link //use the orgin dir, in my case is 2
> --> xfrm_gen_index //generate policy index, there is 0x6E6BB6
>
> then xfrm_policy_timer be fired
>
> xfrm_policy_timer
> --> xfrm_policy_id2dir //get dir from (policy index & 7), in my case is 6
> --> xfrm_policy_delete
> --> __xfrm_policy_unlink //access policy_count[dir], trigger out of range access
>
> Add xfrm_policy_id2dir check in verify_newpolicy_info, make sure the computed dir is
> valid, to fix the issue.
>
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Fixes: e682adf021be ("xfrm: Try to honor policy index if it's supplied by user")
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> ---
> net/xfrm/xfrm_user.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH][next] rsi: fix missing null pointer check from a failed ieee80211_probereq_get call
From: Kalle Valo @ 2019-02-28 8:29 UTC (permalink / raw)
To: Colin King
Cc: Amitkumar Karwar, Siva Rebbagondla, David S . Miller,
linux-wireless, netdev, kernel-janitors, linux-kernel
In-Reply-To: <20190221123549.7294-1-colin.king@canonical.com>
Colin King <colin.king@canonical.com> wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> The call to ieee80211_probereq_get can return NULL if a skb allocation
> fails, so add a null pointer check and free an earlier skb on the error
> exit return path.
>
> Fixes: 7fdcb8e12660 ("rsi: add support for hardware scan offload")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Patch applied to wireless-drivers-next.git, thanks.
74ee436f9581 rsi: fix missing null pointer check from a failed ieee80211_probereq_get call
--
https://patchwork.kernel.org/patch/10824023/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH 3/5] mt76: fix a leaked reference by adding a missing of_node_put
From: Kalle Valo @ 2019-02-28 8:39 UTC (permalink / raw)
To: Wen Yang
Cc: linus.walleij, andrew, vivien.didelot, f.fainelli, davem, netdev,
linux-kernel, alexandre.belloni, UNGLinuxDriver, nbd,
lorenzo.bianconi83, matthias.bgg, linux-wireless,
linux-arm-kernel, linux-mediatek, anirudh, John.Linn,
michal.simek, wang.yi59, Wen Yang
In-Reply-To: <1550819742-32155-3-git-send-email-wen.yang99@zte.com.cn>
Wen Yang <wen.yang99@zte.com.cn> wrote:
> The call to of_find_node_by_phandle returns a node pointer with refcount
> incremented thus it must be explicitly decremented after the last
> usage.
>
> Detected by coccinelle with the following warnings:
> ./drivers/net/wireless/mediatek/mt76/eeprom.c:58:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 48, but without a corresponding object release within this function.
> ./drivers/net/wireless/mediatek/mt76/eeprom.c:61:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 48, but without a corresponding object release within this function.
> ./drivers/net/wireless/mediatek/mt76/eeprom.c:67:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 48, but without a corresponding object release within this function.
> ./drivers/net/wireless/mediatek/mt76/eeprom.c:70:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 48, but without a corresponding object release within this function.
> ./drivers/net/wireless/mediatek/mt76/eeprom.c:72:1-7: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 48, but without a corresponding object release within this function.
>
> Signed-off-by: Wen Yang <wen.yang99@zte.com.cn>
> Cc: Felix Fietkau <nbd@nbd.name>
> Cc: Lorenzo Bianconi <lorenzo.bianconi83@gmail.com>
> Cc: Kalle Valo <kvalo@codeaurora.org>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Matthias Brugger <matthias.bgg@gmail.com>
> Cc: linux-wireless@vger.kernel.org
> Cc: netdev@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-mediatek@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
Patch applied to wireless-drivers-next.git, thanks.
34e022d8b780 mt76: fix a leaked reference by adding a missing of_node_put
--
https://patchwork.kernel.org/patch/10825315/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH] rtlwifi: rtl8723be: Remove set but not used variable 'b_last_is_cur_rdlstate'
From: Kalle Valo @ 2019-02-28 8:39 UTC (permalink / raw)
To: YueHaibing
Cc: Ping-Ke Shih, Larry Finger, YueHaibing, linux-wireless, netdev,
kernel-janitors
In-Reply-To: <20190227022321.7623-1-yuehaibing@huawei.com>
YueHaibing <yuehaibing@huawei.com> wrote:
> Fixes gcc '-Wunused-but-set-variable' warning:
>
> drivers/net/wireless/realtek/rtlwifi/rtl8723be/dm.c: In function 'rtl8723be_dm_check_edca_turbo':
> drivers/net/wireless/realtek/rtlwifi/rtl8723be/dm.c:998:7: warning:
> variable 'b_last_is_cur_rdlstate' set but not used [-Wunused-but-set-variable]
>
> It's never used and can be removed.
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Patch applied to wireless-drivers-next.git, thanks.
f32150565601 rtlwifi: rtl8723be: Remove set but not used variable 'b_last_is_cur_rdlstate'
--
https://patchwork.kernel.org/patch/10830999/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH] rtlwifi: rtl8723ae: Remove set but not used variable 'bt_retry_cnt'
From: Kalle Valo @ 2019-02-28 8:40 UTC (permalink / raw)
To: YueHaibing
Cc: Ping-Ke Shih, Larry Finger, YueHaibing, linux-wireless, netdev,
kernel-janitors
In-Reply-To: <20190227022334.7750-1-yuehaibing@huawei.com>
YueHaibing <yuehaibing@huawei.com> wrote:
> Fixes gcc '-Wunused-but-set-variable' warning:
>
> drivers/net/wireless/realtek/rtlwifi/rtl8723ae/hal_btc.c: In function '_rtl8723e_dm_bt_coexist_2_ant':
> drivers/net/wireless/realtek/rtlwifi/rtl8723ae/hal_btc.c:1408:5: warning:
> variable 'bt_retry_cnt' set but not used [-Wunused-but-set-variable]
>
> It's never used and can be removed.
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Patch applied to wireless-drivers-next.git, thanks.
85c6ac33c87c rtlwifi: rtl8723ae: Remove set but not used variable 'bt_retry_cnt'
--
https://patchwork.kernel.org/patch/10831001/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH 1/8] net/mlx4: use kzalloc instead of kmalloc
From: Tariq Toukan @ 2019-02-28 8:40 UTC (permalink / raw)
To: Robert Eshleman
Cc: Tariq Toukan, David S. Miller, netdev@vger.kernel.org,
linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <b350ebb5410291e80e12244ccb41b4b4bae31e0c.1551246708.git.bobbyeshleman@gmail.com>
On 2/27/2019 8:09 AM, Robert Eshleman wrote:
> This patch replaces a kmalloc/memset(,0) call with a call to kzalloc.
> It also removes a memset(,0) call that always follows a *zalloc call.
>
> Signed-off-by: Robert Eshleman <bobbyeshleman@gmail.com>
> ---
> drivers/net/ethernet/mellanox/mlx4/cmd.c | 1 -
> drivers/net/ethernet/mellanox/mlx4/en_rx.c | 3 +--
> 2 files changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/cmd.c b/drivers/net/ethernet/mellanox/mlx4/cmd.c
> index e65bc3c95630..7bfa6e850e5f 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/cmd.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/cmd.c
> @@ -3307,7 +3307,6 @@ int mlx4_get_counter_stats(struct mlx4_dev *dev, int counter_index,
> if (IS_ERR(mailbox))
> return PTR_ERR(mailbox);
>
> - memset(mailbox->buf, 0, sizeof(struct mlx4_counter));
> if_stat_in_mod = counter_index;
> if (reset)
> if_stat_in_mod |= MLX4_QUERY_IF_STAT_RESET;
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
> index 9a0881cb7f51..f55805d206ef 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
> @@ -1044,7 +1044,7 @@ static int mlx4_en_config_rss_qp(struct mlx4_en_priv *priv, int qpn,
> struct mlx4_qp_context *context;
> int err = 0;
>
> - context = kmalloc(sizeof(*context), GFP_KERNEL);
> + context = kzalloc(sizeof(*context), GFP_KERNEL);
> if (!context)
> return -ENOMEM;
>
> @@ -1055,7 +1055,6 @@ static int mlx4_en_config_rss_qp(struct mlx4_en_priv *priv, int qpn,
> }
> qp->event = mlx4_en_sqp_event;
>
> - memset(context, 0, sizeof(*context));
> mlx4_en_fill_qp_context(priv, ring->actual_size, ring->stride, 0, 0,
> qpn, ring->cqn, -1, context);
> context->db_rec_addr = cpu_to_be64(ring->wqres.db.dma);
>
Hi,
Thanks for your patch.
It looks good, but misses one similar point you might want to cover:
in drivers/net/ethernet/mellanox/mlx4/port.c:1780 :
memset(context, 0, sizeof(*context));
Tariq
^ permalink raw reply
* Re: [PATCH] rtlwifi: rtl8192se: Remove set but not used variable 'seg_ptr'
From: Kalle Valo @ 2019-02-28 8:40 UTC (permalink / raw)
To: YueHaibing
Cc: Ping-Ke Shih, Larry Finger, YueHaibing, linux-wireless, netdev,
kernel-janitors
In-Reply-To: <20190227022347.7927-1-yuehaibing@huawei.com>
YueHaibing <yuehaibing@huawei.com> wrote:
> Fixes gcc '-Wunused-but-set-variable' warning:
>
> drivers/net/wireless/realtek/rtlwifi/rtl8192se/fw.c: In function '_rtl92s_firmware_downloadcode':
> drivers/net/wireless/realtek/rtlwifi/rtl8192se/fw.c:139:17: warning:
> variable 'seg_ptr' set but not used [-Wunused-but-set-variable]
>
> It's not used after commit 59ae1d127ac0 ("networking: introduce and use
> skb_put_data()")
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Patch applied to wireless-drivers-next.git, thanks.
037e0c5df649 rtlwifi: rtl8192se: Remove set but not used variable 'seg_ptr'
--
https://patchwork.kernel.org/patch/10831003/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH net-next 2/8] devlink: add PF and VF port flavours
From: Jiri Pirko @ 2019-02-28 8:44 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: davem, oss-drivers, netdev
In-Reply-To: <20190227144239.55481885@cakuba.netronome.com>
Wed, Feb 27, 2019 at 11:42:39PM CET, jakub.kicinski@netronome.com wrote:
>On Wed, 27 Feb 2019 21:17:27 +0100, Jiri Pirko wrote:
>> Wed, Feb 27, 2019 at 06:23:26PM CET, jakub.kicinski@netronome.com wrote:
>> >On Wed, 27 Feb 2019 13:41:35 +0100, Jiri Pirko wrote:
>> >> Wed, Feb 27, 2019 at 01:23:27PM CET, jiri@resnulli.us wrote:
>> >> >Tue, Feb 26, 2019 at 07:24:30PM CET, jakub.kicinski@netronome.com wrote:
>> >> >>Current port flavours cover simple switches and DSA. Add PF
>> >> >>and VF flavours to cover "switchdev" SR-IOV NICs.
>> >> >>
>> >> >>Example devlink user space output:
>> >> >>
>> >> >>$ devlink port
>> >> >>pci/0000:82:00.0/0: type eth netdev p4p1 flavour physical
>> >> >>pci/0000:82:00.0/10000: type eth netdev eth0 flavour pcie_pf pf 0
>> >> >>pci/0000:82:00.0/10001: type eth netdev eth1 flavour pcie_vf pf 0 vf 0
>> >> >>pci/0000:82:00.0/10002: type eth netdev eth2 flavour pcie_vf pf 0 vf 1
>> >> >
>> >> >Wait a second, howcome pf and vfs have the same PCI address?
>> >>
>> >> Oh, I think you have these as eswitch port representors. Confusing...
>> >
>> >FWIW I don't like the word representor, its a port. We don't call
>> >physical ports "representors" even though from ASIC's point of view
>> >they are exactly the same.
>>
>> My point is, they are not PFs and VFs. We have to find a way to clearly
>> see what's what.
>
>Okay, so let me explain the way I see it, and you can explain your way
>or tell me where you disagree. Those devlink ports and netdevs are pf
>ports and vf ports, which most refer to as "representor". If one sends
>packets to the netdev indicated in DEVLINK_ATTR_PORT_NETDEV_*
>attributes they will _egress_ the switch from that port. For physical
>port that means going onto the Ethernet or IB wire. For PCIe it means
>getting DMAed over the PCIe link to host memory.
>
>There is a netdev construct on the host which is in charge of that
>host memory. Maybe we shall call that host netdev?
>
>(I said I don't like "representor" for the reason that people don't
>refer to the physical port as "representor" even though it has exactly
>the semantics we are following. This distinction between behaviour of
>physical and PCI ports is what leads to confusion, I think.)
>
>Let me bring out the moose :)
>
> HOST A || HOST B
> ||
> PF A | V | V | V | V || PF B | V | V | V
> |*F |*F |*F |*F ... || |*F |*F |*F ...
>*port A0 |*port A1 | 0 | 1 | 2 | 3 ||*port B0 |*port B1 | 0 | 1 | 2
> ||
> PCI Express link || PCI Express link
> \ \ \ | | | | | / / /
> \ \ \ | | | | | / / /
> /\ \______\______\'___|___|__________|_______'____/___/___/__ /\
> || |+PF0s0|+PF0s1 |+VF0|+VF1| ...| |+PF1s0|+PF1s1|+VF0|+VF1| ||
> i || |------ ------ ----- ---- ----|--- ------ ------ ---- ----| || i
>d n H || | <<========== | || d n H
>e s O || | ==========>> | || e s O
>v t S || | SR-IOV e-switch | || v t S
>l a T || | <<========== | || l a T
>i n || | ==========>> | || i n
>n c A || | ________ _________ ________ | || n c B
>k e || | |+Phys 0 |+Phys 1 |+Phys 2 | | || k e
> || \---------------------------------------------------------/ ||
> \/ | | | \/
> | | |
> || ||
> MAC 0 || MAC 1 || MAC 2
> || ||
>
>Things marked with + are devlink ports and have port (-repr-) netdevs
>(including physical ports).
>Things marked with * are host netdevs, don't have devlink ports.
Okay, I got it. So you say that devlink ports should always be only
ports of eswitch.
PF host netdev should have "devlink port" instance, correct?
But it still "belongs" under the ASIC represented by the devlink
instance...
^ permalink raw reply
* Problem of rate limit with VXLAN offload Mellanox ConnectX-5 NIC
From: Jingrong Chen @ 2019-02-28 8:55 UTC (permalink / raw)
To: netdev, nic-support, jzhangcs, shuaa
Hi all,
We ran into a performance issue when configuring the ConnectX-5 NICs to do both tc rate limiting and VXLAN encapsulation/decapsulation simultaneously.
Our test setting is as follows:
We have two servers, each installed one ConnectX-5 NIC. Each server runs one VM. We use ib_write_bw to let one VM send RDMA traffic to the other VM. The traffic priority is configured to be 3.
1) We configure tc rate limiting using command "mlnx_qos -i rdma0 -r 0,0,0,X,0,0,0,0" (i.e., limit the traffic rate to be X Gbps).
2) We enable SR-IOV at the NIC and follow the ASAP2 Hardware Offloading for vSwitches User Manual to configure vxlan encapsulation/decapsulation for the RDMA traffic.
Our test result is as below:
Rate limit (Gbps) Throughput (Gbps)
X = 40 19.09 (throughput only achieved half of the configured rate)
X = 60 28.66
X = 80 38.25
X = 100 47.77
unlimited 87.68
* Note that when only enabling tc rate limiting, there is no such throughput downgrade issue.
It would be of great help if anyone could help us to resolve this problem. Thanks.
FYI, our machines run Ubuntu 18.04, the kernel version is 4.15.0, CX5 firmware is 16.24.1000, and the driver is MLNX_OFED_LINUX-4.5-1.0.1.0.
Regards,
Jingrong
^ permalink raw reply
* Re: linux-next: manual merge of the net-next tree with the rdma tree
From: Leon Romanovsky @ 2019-02-28 9:05 UTC (permalink / raw)
To: Doug Ledford
Cc: Stephen Rothwell, David Miller, Networking, Jason Gunthorpe,
Linux Next Mailing List, Linux Kernel Mailing List,
Shamir Rabinovitch, Jakub Kicinski
In-Reply-To: <16f0ac6227f948165b7062a72e1fa82f1dfe4697.camel@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 1194 bytes --]
On Wed, Feb 27, 2019 at 11:24:51AM -0500, Doug Ledford wrote:
> On Wed, 2019-02-27 at 11:25 +1100, Stephen Rothwell wrote:
> > Hi all,
> >
> > Today's linux-next merge of the net-next tree got a conflict in:
> >
> > drivers/infiniband/hw/mlx4/Kconfig
> >
> > between commit:
> >
> > 6fa8f1afd337 ("IB/{core,uverbs}: Move ib_umem_xxx functions from ib_core to ib_uverbs")
> >
> > from the rdma tree and commit:
> >
> > f4b6bcc7002f ("net: devlink: turn devlink into a built-in")
> >
> > from the net-next tree.
> >
> > I fixed it up (see below) 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.
> >
>
> Thanks Stephen, we'll add it to the (largish this release) list of
> conflicts to bring to Linus' attention.
Thanks
>
> --
> Doug Ledford <dledford@redhat.com>
> GPG KeyID: B826A3330E572FDD
> Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: [PATCH net-next 4/8] devlink: allow subports on devlink PCI ports
From: Jiri Pirko @ 2019-02-28 8:56 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: davem, oss-drivers, netdev, parav, jgg
In-Reply-To: <20190227103000.6ea6f7c0@cakuba.netronome.com>
Wed, Feb 27, 2019 at 07:30:00PM CET, jakub.kicinski@netronome.com wrote:
>On Wed, 27 Feb 2019 13:37:53 +0100, Jiri Pirko wrote:
>> Tue, Feb 26, 2019 at 07:24:32PM CET, jakub.kicinski@netronome.com wrote:
>> >PCI endpoint corresponds to a PCI device, but such device
>> >can have one more more logical device ports associated with it.
>> >We need a way to distinguish those. Add a PCI subport in the
>> >dumps and print the info in phys_port_name appropriately.
>> >
>> >This is not equivalent to port splitting, there is no split
>> >group. It's just a way of representing multiple netdevs on
>> >a single PCI function.
>> >
>> >Note that the quality of being multiport pertains only to
>> >the PCI function itself. A PF having multiple netdevs does
>> >not mean that its VFs will also have multiple, or that VFs
>> >are associated with any particular port of a multiport VF.
>>
>> We've been discussing the problem of subport (we call it "subfunction"
>> or "SF") for some time internally. Turned out, this is probably harder
>> task to model. Please prove me wrong.
>>
>> The nature of VF makes it a logically separate entity. It has a separate
>> PCI address, it should therefore have a separate devlink instance.
>> You can pass it through to VM, then the same devlink instance should be
>> created inside the VM and disappear from the host.
>
>Depends what a devlink instance represents :/ On one hand you may want
>to create an instance for a VF to allow it to spawn soft ports, on the
>other you may want to group multiple functions together.
>
>IOW if devlink instance is for an ASIC, there should be one per device
>per host. So if we start connecting multiple functions (PFs and/or VFs)
>to one host we should probably introduce the notion of devlink aliases
>or some such (so that multiple bus addresses can target the same
Hmm. Like VF address -> PF address alias? That would be confusing to see
eswitch ports under VF devlink instance... I probably did not get you
right.
>devlink instance). Those less pipelined NICs can forward between
>ports, but still want a function per port (otherwise user space
>sometimes gets confused). If we have multiple functions which are on
>the same "switchid" they should have a single devlink instance if you
>ask me. That instance will have all the ports of the device.
Okay, that makes sense. But the question it, can the same devlink
instance contain ports that does not have "Switchid"?
I think it would be beneficial to have the switchid shown for devlink
ports too. Then it is clean that the devlink ports with the same
switchid belong to the same switch, and other ports under the same
devlink instance (like PF itself) is separate, but still under the same
ASIC.
>
>You say disappear from the host - what do you mean. Are you referring
>to the VF port disappearing? But on the switch the port is still
No, VF itself. eswitch port will be still there on the host.
>there, and you should show the subports on the PF side IMHO. Devlink
>ports should allow users to understand the topology of the switch.
What do you mean by "topology"?
>
>Is spawning VMDq sub-instances the only thing we can think of that VMs
>may want to do? Are there any other uses?
>
>> SF (or subport) feels similar to that. Basically it is exactly the same
>> thing as VF, only does reside under PF PCI function.
>>
>> That is why I think, for sake of consistency, it should have a separate
>> devlink entity as well. The problem is correct sysfs modelling and
>> devlink handle derived from that. Parav is working on a simple soft
>> bus for this purpose called "subbus". There is a RFC floating around on
>> Mellanox internal mailing list, looks like it is time to send it
>> upstream.
>>
>> Then each PF driver which have SFs would register subbus devices
>> according to SFs/subports and they would be properly handled by bus
>> probe, devlink and devlink port and netdev instances created.
>>
>> Ccing Parav and Jason.
>
>You guys come from the RDMA side of the world, with which I'm less
>familiar, and the soft bus + spawning devices seems to be a popular
>design there. Could you describe the advantages of that model for
>the sake of the netdev-only folks? :)
I'll try to draw some ascii art :)
>
>Another term that gets thrown into the mix here is mediated devices,
>right? If you wanna pass the sub-spawn-soft-port to a VM. Or run
>DPDK on some queues.
>
>To state the obvious AF_XDP and macvlan offload were are previous
>answers to some of those use cases. What is the forwarding model
>for those subports? Are we going to allow flower rules from VMs?
>Is it going to be dst MAC only? Or is the hypervisor going to forward
>as it sees appropriate (OvS + "repr"/port netdev)?
^ permalink raw reply
* Re: [PATCH RFC] mac80211: Use IFF_ECHO to force delivery of tx_status frames
From: Julius Niedworok @ 2019-02-28 9:05 UTC (permalink / raw)
To: Johannes Berg
Cc: Oliver Hartkopp, linux-wireless, ga58taw, David Hildenbrand, nc,
David S. Miller, Edward Cree, Jiri Pirko, Ido Schimmel,
Petr Machata, Kirill Tkhai, Alexander Duyck, Amritha Nambiar,
Li RongQing, netdev, linux-kernel, Julius Niedworok
In-Reply-To: <550ff2ac45c891a3fcf3dd8a454f9732d8aa1b70.camel@sipsolutions.net>
> On 26.02.2019 14:33 Johannes Berg wrote
>
> You're proposing to add this to the *monitor* interfaces and you really
> should have made the flag conditional on that to make that clear.
>
> However, even on monitor interfaces, you typically *already* see the
> frames you transmitted there (as raw frames, which is the only thing you
> can do).
>
Thanks for your prompt reply and thoughts on our patch.
Let us briefly describe our test setup to ensure everyone on this mailing
list is one the same page.
Our general setup looks like this:
1 $ iw wlp1s0 info
Interface wlp1s0
ifindex 5
wdev 0x1
addr 4c:5e:0c:11:43:ac
type managed
wiphy 0
txpower 30.00 dBm
1 $ iw phy phy0 interface add mon0 type monitor
1 $ iw phy phy0 interface add mon1 type monitor
When we send (raw) packets on mon0 using packetspammer [1] and listen on
the _other_ monitor mode interface mon1, we receive frames that were sent
on the first one:
1 $ packetspammer mon0
2 $ tcpdump -i mon1 'wlan addr2 13:22:33:44:55:66'
This is due to the fact that frames sent on mon0 are echoed back as TX
status frames, because REQ_TX_STATUS is always set for frames sent from
monitor mode interfaces.
But when we replace mon0 with an interface in managed mode (wlp1s0), the
receipt of frames stops, because in managed mode REQ_TX_STATUS is cleared
in most frames:
1 $ ifup wlp1s0
1 $ ping -I wlp1s0 192.168.254.1 # this address is not assigned to any host
2 $ tcpdump -i mon1 ‚wlan addr2 4c:5e:0c:11:43:ac‘
> What you're proposing is to use IFF_ECHO to show frames transmitted
> through *other* interfaces on the monitor interface.
>
> I don’t think the IFF_ECHO semantics really match this.
>
What we propose is to use IFF_ECHO to force REQ_TX_STATUS being set for all
frames sent on the interface. But you are right: The goal is that frames
transmitted through the other interface show up on the monitor interface
(but only after passing the driver). However, this is exactly how we
understand the semantics of IFF_ECHO in the kernel documentation.
>
> Additionally, drivers are sort of free to ignore the REQ_TX_STATUS, or
> we could in the future add ways of using the _noskb to feed back TX
> status to the state machines where needed, so I'm not really sure I even
> _want_ this to be set in stone in such an API.
>
As far as we know, drivers must return a TX status frame, if REQ_TX_STATUS
is set, but can do whatever they want, if it is clear. This is no problem for our
functionality, because we force the delivery of TX status frames by
permanently setting REQ_TX_STATUS. As long as the semantics of
REQ_TX_STATUS remains like it is now, the functionality will always be
as expected from our API.
> Now, I can also see how this can be useful for debugging, but it feels
> to me like this should be a driver (debug) option?
>
We could also achieve the functionality by modifying the drivers but this
would mean that we had to add this functionality to every driver.
Moreover, the feature of TX status frames, how it is implemented currently
for monitor mode interfaces, is part of the mac80211 implementation. The
decision to force TX status frames for monitor mode interfaces is made in
the common mac80211 implementation.
> johannes
>
Once again, thanks for your comments. We appreciate your response.
Julius and Charlie
[1] https://warmcat.com/git/packetspammer
^ permalink raw reply
* Re: [PATCH net-next 6/8] devlink: introduce port's peer netdevs
From: Jiri Pirko @ 2019-02-28 9:00 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: davem, oss-drivers, netdev
In-Reply-To: <20190227104742.3897ae1a@cakuba.netronome.com>
Wed, Feb 27, 2019 at 07:47:42PM CET, jakub.kicinski@netronome.com wrote:
>On Wed, 27 Feb 2019 14:08:29 +0100, Jiri Pirko wrote:
>> Tue, Feb 26, 2019 at 07:24:34PM CET, jakub.kicinski@netronome.com wrote:
>> >Devlink ports represent ports of a switch device (or SR-IOV
>> >NIC which has an embedded switch). In case of SR-IOV when
>> >PCIe PFs are exposed the PFs which are directly connected
>> >to the local machine may also spawn PF netdev (much like
>> >VFs have a port/"repr" and an actual VF netdev).
>> >
>> >Allow devlink to expose such linking. There is currently no
>> >way to find out which netdev corresponds to which PF.
>> >
>> >Example:
>> >
>> >$ devlink port
>> >pci/0000:82:00.0/0: type eth netdev p4p1 flavour physical
>> >pci/0000:82:00.0/10000: type eth netdev eth1 flavour pci_pf pf 0 peer_netdev enp130s0
>> >pci/0000:82:00.0/10001: type eth netdev eth0 flavour pci_vf pf 0 vf 0
>> >pci/0000:82:00.0/10002: type eth netdev eth2 flavour pci_vf pf 0 vf 1
>>
>> Peer as the other side of a "virtual cable". For PF, that is probably
>> sufficient. But I think what a "peer of devlink port" should be "a
>> devlink port".
>
>Maybe I'm not clear on what devlink port is - to me its a port of the
>ASIC. The notion of devlink port connected to devlink port seems
>to counter such definition :S
"port of the ASIC" in a sence of "eswitch ports"?
>
>I do not think that every netdev should have a devlink port associated.
>
>> Not sure about VF.
>>
>> Consider a simple problem of setting up a VF mac address. In legacy, you
>> do it like this:
>> $ ip link set eth2 vf 1 mac 00:52:44:11:22:33
>> However, in new model, you so far cannot do that.
>
>Why?
>
>$ devlink port set pci/0000:82:00.0/10001 peer_eth_addr 00:52:44:11:22:33
Yeah. That is not yet implemented. I agree it is most straightforward.
The question is, is it fine to have set of:
peer_eth_addr
peer_mtu
peer_something_else
Or rather to have some object to pin this on. Something like:
$ devlink port peer set pci/0000:82:00.0/10001 eth_addr 00:52:44:11:22:33
>
>It's more of a neighbour info situation than a local port situation.
>
>> What I was thinking about was some "dummy peer" which would be on the
>> host. Not sure if only as a "dummy peer devlink port" or even as some
>> sort of "dummy netdev".
>>
>> One way or another, it would provide the user some info about which VF
>> representor is connected to which VF in VM (mac mapping).
>
>Ack, but isn't the MAC setting is the only thing we're missing from
>"switchdev SR-IOV"? Would the "dummy netdev" be used for anything
>else? I would rather not introduce new netdev just to do that
Agreed. It was just a wild idea :)
>(that'd be a third for that port.)
^ permalink raw reply
* [PATCH] net: e1000e: add MAC address kernel cmd line parameter
From: Flavio Suligoi @ 2019-02-28 9:20 UTC (permalink / raw)
To: Jeff Kirsher, David S . Miller, intel-wired-lan, netdev,
linux-kernel
Cc: Flavio Suligoi
Sometimes, in some embedded systems boards (i.e. ARM boards),
the NVM eeprom is not mounted, to save cost and space.
In this case it is necessary to bypass the NVM management
and directly force the MAC address using a kernel command-line
parameter (macaddr).
Signed-off-by: Flavio Suligoi <f.suligoi@asem.it>
---
drivers/net/ethernet/intel/e1000e/netdev.c | 50 ++++++++++++++++++++----------
1 file changed, 33 insertions(+), 17 deletions(-)
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index 189f231..054c398 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -39,6 +39,10 @@ static int debug = -1;
module_param(debug, int, 0);
MODULE_PARM_DESC(debug, "Debug level (0=none,...,16=all)");
+static unsigned char macaddr[ETH_ALEN] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
+module_param_array(macaddr, byte, NULL, 0);
+MODULE_PARM_DESC(macaddr, "e1000e Ethernet MAC address");
+
static const struct e1000_info *e1000_info_tbl[] = {
[board_82571] = &e1000_82571_info,
[board_82572] = &e1000_82572_info,
@@ -7053,6 +7057,7 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
static int cards_found;
u16 aspm_disable_flag = 0;
int bars, i, err, pci_using_dac;
+
u16 eeprom_data = 0;
u16 eeprom_apme_mask = E1000_EEPROM_APME;
s32 ret_val = 0;
@@ -7232,27 +7237,38 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
*/
adapter->hw.mac.ops.reset_hw(&adapter->hw);
- /* systems with ASPM and others may see the checksum fail on the first
- * attempt. Let's give it a few tries
- */
- for (i = 0;; i++) {
- if (e1000_validate_nvm_checksum(&adapter->hw) >= 0)
- break;
- if (i == 2) {
- dev_err(&pdev->dev, "The NVM Checksum Is Not Valid\n");
- err = -EIO;
- goto err_eeprom;
+ /* If the MAC address is supplied by the bootloader, use it! */
+ if (!is_multicast_ether_addr(macaddr)) {
+ dev_info(&pdev->dev,
+ "MAC address from kernel command line argument\n");
+ memcpy(adapter->hw.mac.addr, macaddr, netdev->addr_len);
+ memcpy(netdev->dev_addr, macaddr, netdev->addr_len);
+ } else {
+ /* systems with ASPM and others may see the checksum fail on
+ * the first attempt. Let's give it a few tries
+ */
+ dev_info(&pdev->dev, "MAC address from NVM\n");
+ for (i = 0;; i++) {
+ if (e1000_validate_nvm_checksum(&adapter->hw) >= 0)
+ break;
+ if (i == 2) {
+ dev_err(&pdev->dev,
+ "The NVM Checksum Is Not Valid\n");
+ err = -EIO;
+ goto err_eeprom;
+ }
}
- }
- e1000_eeprom_checks(adapter);
+ e1000_eeprom_checks(adapter);
- /* copy the MAC address */
- if (e1000e_read_mac_addr(&adapter->hw))
- dev_err(&pdev->dev,
- "NVM Read Error while reading MAC address\n");
+ /* copy the MAC address */
+ if (e1000e_read_mac_addr(&adapter->hw))
+ dev_err(&pdev->dev,
+ "NVM Read Error while reading MAC address\n");
- memcpy(netdev->dev_addr, adapter->hw.mac.addr, netdev->addr_len);
+ memcpy(netdev->dev_addr, adapter->hw.mac.addr,
+ netdev->addr_len);
+ }
if (!is_valid_ether_addr(netdev->dev_addr)) {
dev_err(&pdev->dev, "Invalid MAC Address: %pM\n",
--
2.7.4
^ permalink raw reply related
* Re: [virtio-dev] Re: net_failover slave udev renaming (was Re: [RFC PATCH net-next v6 4/4] netvsc: refactor notifier/event handling code to use the bypass framework)
From: si-wei liu @ 2019-02-28 9:32 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Samudrala, Sridhar, Siwei Liu, Jiri Pirko, Stephen Hemminger,
David Miller, Netdev, virtualization, virtio-dev,
Brandeburg, Jesse, Alexander Duyck, Jakub Kicinski, Jason Wang,
liran.alon
In-Reply-To: <20190227193923-mutt-send-email-mst@kernel.org>
On 2/27/2019 4:41 PM, Michael S. Tsirkin wrote:
> On Wed, Feb 27, 2019 at 04:38:00PM -0800, si-wei liu wrote:
>>
>> On 2/27/2019 3:50 PM, Michael S. Tsirkin wrote:
>>> On Wed, Feb 27, 2019 at 03:34:56PM -0800, si-wei liu wrote:
>>>> On 2/27/2019 2:38 PM, Michael S. Tsirkin wrote:
>>>>> On Tue, Feb 26, 2019 at 04:17:21PM -0800, si-wei liu wrote:
>>>>>> On 2/25/2019 6:08 PM, Michael S. Tsirkin wrote:
>>>>>>> On Mon, Feb 25, 2019 at 04:58:07PM -0800, si-wei liu wrote:
>>>>>>>> On 2/22/2019 7:14 AM, Michael S. Tsirkin wrote:
>>>>>>>>> On Thu, Feb 21, 2019 at 11:55:11PM -0800, si-wei liu wrote:
>>>>>>>>>> On 2/21/2019 11:00 PM, Samudrala, Sridhar wrote:
>>>>>>>>>>> On 2/21/2019 7:33 PM, si-wei liu wrote:
>>>>>>>>>>>> On 2/21/2019 5:39 PM, Michael S. Tsirkin wrote:
>>>>>>>>>>>>> On Thu, Feb 21, 2019 at 05:14:44PM -0800, Siwei Liu wrote:
>>>>>>>>>>>>>> Sorry for replying to this ancient thread. There was some remaining
>>>>>>>>>>>>>> issue that I don't think the initial net_failover patch got addressed
>>>>>>>>>>>>>> cleanly, see:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1815268
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> The renaming of 'eth0' to 'ens4' fails because the udev userspace was
>>>>>>>>>>>>>> not specifically writtten for such kernel automatic enslavement.
>>>>>>>>>>>>>> Specifically, if it is a bond or team, the slave would typically get
>>>>>>>>>>>>>> renamed *before* virtual device gets created, that's what udev can
>>>>>>>>>>>>>> control (without getting netdev opened early by the other part of
>>>>>>>>>>>>>> kernel) and other userspace components for e.g. initramfs,
>>>>>>>>>>>>>> init-scripts can coordinate well in between. The in-kernel
>>>>>>>>>>>>>> auto-enslavement of net_failover breaks this userspace convention,
>>>>>>>>>>>>>> which don't provides a solution if user care about consistent naming
>>>>>>>>>>>>>> on the slave netdevs specifically.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Previously this issue had been specifically called out when IFF_HIDDEN
>>>>>>>>>>>>>> and the 1-netdev was proposed, but no one gives out a solution to this
>>>>>>>>>>>>>> problem ever since. Please share your mind how to proceed and solve
>>>>>>>>>>>>>> this userspace issue if netdev does not welcome a 1-netdev model.
>>>>>>>>>>>>> Above says:
>>>>>>>>>>>>>
>>>>>>>>>>>>> there's no motivation in the systemd/udevd community at
>>>>>>>>>>>>> this point to refactor the rename logic and make it work well with
>>>>>>>>>>>>> 3-netdev.
>>>>>>>>>>>>>
>>>>>>>>>>>>> What would the fix be? Skip slave devices?
>>>>>>>>>>>>>
>>>>>>>>>>>> There's nothing user can get if just skipping slave devices - the
>>>>>>>>>>>> name is still unchanged and unpredictable e.g. eth0, or eth1 the
>>>>>>>>>>>> next reboot, while the rest may conform to the naming scheme (ens3
>>>>>>>>>>>> and such). There's no way one can fix this in userspace alone - when
>>>>>>>>>>>> the failover is created the enslaved netdev was opened by the kernel
>>>>>>>>>>>> earlier than the userspace is made aware of, and there's no
>>>>>>>>>>>> negotiation protocol for kernel to know when userspace has done
>>>>>>>>>>>> initial renaming of the interface. I would expect netdev list should
>>>>>>>>>>>> at least provide the direction in general for how this can be
>>>>>>>>>>>> solved...
>>>>>>>>> I was just wondering what did you mean when you said
>>>>>>>>> "refactor the rename logic and make it work well with 3-netdev" -
>>>>>>>>> was there a proposal udev rejected?
>>>>>>>> No. I never believed this particular issue can be fixed in userspace alone.
>>>>>>>> Previously someone had said it could be, but I never see any work or
>>>>>>>> relevant discussion ever happened in various userspace communities (for e.g.
>>>>>>>> dracut, initramfs-tools, systemd, udev, and NetworkManager). IMHO the root
>>>>>>>> of the issue derives from the kernel, it makes more sense to start from
>>>>>>>> netdev, work out and decide on a solution: see what can be done in the
>>>>>>>> kernel in order to fix it, then after that engage userspace community for
>>>>>>>> the feasibility...
>>>>>>>>
>>>>>>>>> Anyway, can we write a time diagram for what happens in which order that
>>>>>>>>> leads to failure? That would help look for triggers that we can tie
>>>>>>>>> into, or add new ones.
>>>>>>>>>
>>>>>>>> See attached diagram.
>>>>>>>>
>>>>>>>>>>> Is there an issue if slave device names are not predictable? The user/admin scripts are expected
>>>>>>>>>>> to only work with the master failover device.
>>>>>>>>>> Where does this expectation come from?
>>>>>>>>>>
>>>>>>>>>> Admin users may have ethtool or tc configurations that need to deal with
>>>>>>>>>> predictable interface name. Third-party app which was built upon specifying
>>>>>>>>>> certain interface name can't be modified to chase dynamic names.
>>>>>>>>>>
>>>>>>>>>> Specifically, we have pre-canned image that uses ethtool to fine tune VF
>>>>>>>>>> offload settings post boot for specific workload. Those images won't work
>>>>>>>>>> well if the name is constantly changing just after couple rounds of live
>>>>>>>>>> migration.
>>>>>>>>> It should be possible to specify the ethtool configuration on the
>>>>>>>>> master and have it automatically propagated to the slave.
>>>>>>>>>
>>>>>>>>> BTW this is something we should look at IMHO.
>>>>>>>> I was elaborating a few examples that the expectation and assumption that
>>>>>>>> user/admin scripts only deal with master failover device is incorrect. It
>>>>>>>> had never been taken good care of, although I did try to emphasize it from
>>>>>>>> the very beginning.
>>>>>>>>
>>>>>>>> Basically what you said about propagating the ethtool configuration down to
>>>>>>>> the slave is the key pursuance of 1-netdev model. However, what I am seeking
>>>>>>>> now is any alternative that can also fix the specific udev rename problem,
>>>>>>>> before concluding that 1-netdev is the only solution. Generally a 1-netdev
>>>>>>>> scheme would take time to implement, while I'm trying to find a way out to
>>>>>>>> fix this particular naming problem under 3-netdev.
>>>>>>>>
>>>>>>>>>>> Moreover, you were suggesting hiding the lower slave devices anyway. There was some discussion
>>>>>>>>>>> about moving them to a hidden network namespace so that they are not visible from the default namespace.
>>>>>>>>>>> I looked into this sometime back, but did not find the right kernel api to create a network namespace within
>>>>>>>>>>> kernel. If so, we could use this mechanism to simulate a 1-netdev model.
>>>>>>>>>> Yes, that's one possible implementation (IMHO the key is to make 1-netdev
>>>>>>>>>> model as much transparent to a real NIC as possible, while a hidden netns is
>>>>>>>>>> just the vehicle). However, I recall there was resistance around this
>>>>>>>>>> discussion that even the concept of hiding itself is a taboo for Linux
>>>>>>>>>> netdev. I would like to summon potential alternatives before concluding
>>>>>>>>>> 1-netdev is the only solution too soon.
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> -Siwei
>>>>>>>>> Your scripts would not work at all then, right?
>>>>>>>> At this point we don't claim images with such usage as SR-IOV live
>>>>>>>> migrate-able. We would flag it as live migrate-able until this ethtool
>>>>>>>> config issue is fully addressed and a transparent live migration solution
>>>>>>>> emerges in upstream eventually.
>>>>>>>>
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> -Siwei
>>>>>>>>>>>> -Siwei
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>> ---------------------------------------------------------------------
>>>>>>>>> To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
>>>>>>>>> For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
>>>>>>>>>
>>>>>>>> net_failover(kernel) | network.service (user) | systemd-udevd (user)
>>>>>>>> --------------------------------------------------+------------------------------+--------------------------------------------
>>>>>>>> (standby virtio-net and net_failover | |
>>>>>>>> devices created and initialized, | |
>>>>>>>> i.e. virtnet_probe()-> | |
>>>>>>>> net_failover_create() | |
>>>>>>>> was done.) | |
>>>>>>>> | |
>>>>>>>> | runs `ifup ens3' -> |
>>>>>>>> | ip link set dev ens3 up |
>>>>>>>> net_failover_open() | |
>>>>>>>> dev_open(virtnet_dev) | |
>>>>>>>> virtnet_open(virtnet_dev) | |
>>>>>>>> netif_carrier_on(failover_dev) | |
>>>>>>>> ... | |
>>>>>>>> | |
>>>>>>>> (VF hot plugged in) | |
>>>>>>>> ixgbevf_probe() | |
>>>>>>>> register_netdev(ixgbevf_netdev) | |
>>>>>>>> netdev_register_kobject(ixgbevf_netdev) | |
>>>>>>>> kobject_add(ixgbevf_dev) | |
>>>>>>>> device_add(ixgbevf_dev) | |
>>>>>>>> kobject_uevent(&ixgbevf_dev->kobj, KOBJ_ADD) | |
>>>>>>>> netlink_broadcast() | |
>>>>>>>> ... | |
>>>>>>>> call_netdevice_notifiers(NETDEV_REGISTER) | |
>>>>>>>> failover_event(..., NETDEV_REGISTER, ...) | |
>>>>>>>> failover_slave_register(ixgbevf_netdev) | |
>>>>>>>> net_failover_slave_register(ixgbevf_netdev) | |
>>>>>>>> dev_open(ixgbevf_netdev) | |
>>>>>>>> | |
>>>>>>>> | |
>>>>>>>> | | received ADD uevent from netlink fd
>>>>>>>> | | ...
>>>>>>>> | | udev-builtin-net_id.c:dev_pci_slot()
>>>>>>>> | | (decided to renamed 'eth0' )
>>>>>>>> | | ip link set dev eth0 name ens4
>>>>>>>> (dev_change_name() returns -EBUSY as | |
>>>>>>>> ixgbevf_netdev->flags has IFF_UP) | |
>>>>>>>> | |
>>>>>>>>
>>>>>>> Given renaming slaves does not work anyway:
>>>>>> I was actually thinking what if we relieve the rename restriction just for
>>>>>> the failover slave? What the impact would be? I think users don't care about
>>>>>> slave being renamed when it's in use, especially the initial rename.
>>>>>> Thoughts?
>>>>>>
>>>>>>> would it work if we just
>>>>>>> hard-coded slave names instead?
>>>>>>>
>>>>>>> E.g.
>>>>>>> 1. fail slave renames
>>>>>>> 2. rename of failover to XX automatically renames standby to XXnsby
>>>>>>> and primary to XXnpry
>>>>>> That wouldn't help. The time when the failover master gets renamed, the VF
>>>>>> may not be present.
>>>>> In this scheme if VF is not there it will be renamed immediately after registration.
>>>> Who will be responsible to rename the slave, the kernel?
>>> That's the idea.
>>>
>>>> Note the master's
>>>> name may or may not come from the userspace. If it comes from the userspace,
>>>> should the userspace daemon change their expectation not to name/rename
>>>> _any_ slaves (today there's no distinction)?
>>> Yes the idea would be to fail renaming slaves.
>> No I was asking about the userspace expectation: whether it should track and
>> detect the lifecycle events of failover slaves and decide what to do. How
>> does it get back to the user specified name if VF is not enslaved (say
>> someone unloads the virtio-net module)?
> When virtio net is removed VF will shortly be removed too.
>
>> As this scheme adds much complexity to the kernel naming convention
>> (currently it's just ethX names) that no userspace can understand.
> Anything that pokes at slaves needs to be specially designed anyway.
> Naming seems like a minor issue.
>
>> Will the
>> change break userspace further?
>>
>> -Siwei
> Didn't you show userspace is already broken. You can't "further
> break it", rename already fails.
It's a race, userspace tends to give slave a user(space) desired name
but sometimes may fail due to this race. Today if failover master is not
up, rename would succeed anyway. While what you proposed prohibits user
from providing a name in all circumstances if I understand you
correctly. That's what I meant of breaking userspace further. On the
other hand, you seem to tighten the kernel default naming to udev
predictable names, which is derived from only recent systemd-udevd,
while there exists many possible userspace naming schemes out of that.
Users today who deliberately chooses to disable predictable naming
(net.ifnames=0 biosdevname=0) and fall back to kernel provided names
would expect the ethX pattern, with this change admin/user scripts which
matches the ethX pattern could potentially break.
IMHO that change is more risky than allow userspace to change the name
for failover slave in any case. I would refresh everyone's mind that the
target users of net_failover is very specific to the live migration
scenario, who typically don't have profound knowledge to fiddle with the
low level plumbing but just expect to operate on master device directly.
I don't have much concern over the slave netfilter rule brokenness or
whatsoever if just lifting up the rename restriction: the failover slave
naming itself is already unreliable, how can we break those apps relying
on consistent naming further without fixing it in the first place? It
could be just simply two lines of code change, if any net_failover user,
who may break due to this change, would have come here and complained
about the naming issue earlier. IOW at the very least, the change below
shouldn't make the current situation any worse.
Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com>
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1127,7 +1127,8 @@ int dev_change_name(struct net_device *dev, const
char *newname)
BUG_ON(!dev_net(dev));
net = dev_net(dev);
- if (dev->flags & IFF_UP)
+ if (dev->flags & IFF_UP &&
+ !(dev->priv_flags & IFF_FAILOVER_SLAVE))
return -EBUSY;
write_seqcount_begin(&devnet_rename_seq);
Thanks,
-Siwei
>
>>>> How do users know which name to
>>>> trust, depending on which wins the race more often? Say if kernel wants a
>>>> ens3npry name while userspace wants it named as ens4.
>>>>
>>>> -Siwei
>>> With this approach kernel will deny attempts by userspace to rename
>>> slaves. Slaves will always be named XXXnsby and XXnpry. Master renames
>>> will rename both slaves.
>>>
>>> It seems pretty solid to me, the only issue is that in theory userspace
>>> can use a name like XXXnsby for something else. But this seems unlikely.
>>>
>>>
>>>>>> I don't like the idea to delay exposing failover master
>>>>>> until VF is hot plugged in (probably subject to various failures) later.
>>>>>>
>>>>>> Thanks,
>>>>>> -Siwei
>>>>> I agree, this was not what I meant.
>>>>>
^ permalink raw reply
* Re: [net: PATCH] net: mvpp2: disable link IRQ waiting for IDLE frames
From: Russell King - ARM Linux admin @ 2019-02-28 9:36 UTC (permalink / raw)
To: Marcin Wojtas
Cc: linux-kernel, linux-arm-kernel, netdev, antoine.tenart, jaz,
gregory.clement, nadavh, thomas.petazzoni, tn, stefanc, davem
In-Reply-To: <1551289652-7377-1-git-send-email-mw@semihalf.com>
On Wed, Feb 27, 2019 at 06:47:32PM +0100, Marcin Wojtas wrote:
> Current version of the driver was configuring XLG MAC
> in a way to wait 3 IDLE frames before allowing for the
> link-up interrupt to be triggered. This resulted in an
> issue, preventing to detect the link change during RX
> traffic on the interface. Fix that.
What about noise sensitivity? Does this now mean that on boards such as
Macchiatobin Single Shot or Clearfog GT9k where the serdes is connected
directly to a SFP cage, that noise on the serdes lines triggers a link
indication?
>
> Fixes: 4bb043262878 ("net: mvpp2: phylink support")
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> ---
> drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> index 16066c2..f1378f9 100644
> --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> @@ -4532,8 +4532,7 @@ static void mvpp2_xlg_config(struct mvpp2_port *port, unsigned int mode,
> ctrl0 |= MVPP22_XLG_CTRL0_RX_FLOW_CTRL_EN;
>
> ctrl4 &= ~MVPP22_XLG_CTRL4_MACMODSELECT_GMAC;
> - ctrl4 |= MVPP22_XLG_CTRL4_FWD_FC | MVPP22_XLG_CTRL4_FWD_PFC |
> - MVPP22_XLG_CTRL4_EN_IDLE_CHECK;
> + ctrl4 |= MVPP22_XLG_CTRL4_FWD_FC | MVPP22_XLG_CTRL4_FWD_PFC;
>
> writel(ctrl0, port->base + MVPP22_XLG_CTRL0_REG);
> writel(ctrl4, port->base + MVPP22_XLG_CTRL4_REG);
> --
> 2.7.4
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
^ permalink raw reply
* [PATCH net-next] cxgb4/cxgb4vf: Fix up netdev->hw_features
From: Arjun Vynipadath @ 2019-02-28 9:36 UTC (permalink / raw)
To: netdev, davem; +Cc: nirranjan, indranil, dt, Arjun Vynipadath, Vishal Kulkarni
GRO is done by cxgb4/cxgb4vf. Hence set NETIF_F_GRO flag for
both cxgb4/cxgb4vf.
Cleaned up VLAN netdev features in cxgb4vf. Also fixed
NETIF_F_HIGHDMA being set unconditionally for vlan netdev
features.
Signed-off-by: Arjun Vynipadath <arjun@chelsio.com>
Signed-off-by: Vishal Kulkarni <vishal@chelsio.com>
---
drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 4 ++--
drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c | 15 +++++++--------
2 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
index bcbac247a73d..200b16034544 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
@@ -5303,7 +5303,7 @@ static void free_some_resources(struct adapter *adapter)
#define TSO_FLAGS (NETIF_F_TSO | NETIF_F_TSO6 | NETIF_F_TSO_ECN)
#define VLAN_FEAT (NETIF_F_SG | NETIF_F_IP_CSUM | TSO_FLAGS | \
- NETIF_F_IPV6_CSUM | NETIF_F_HIGHDMA)
+ NETIF_F_GRO | NETIF_F_IPV6_CSUM | NETIF_F_HIGHDMA)
#define SEGMENT_SIZE 128
static int t4_get_chip_type(struct adapter *adap, int ver)
@@ -5710,7 +5710,7 @@ static int init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
netdev->hw_features = NETIF_F_SG | TSO_FLAGS |
NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
- NETIF_F_RXCSUM | NETIF_F_RXHASH |
+ NETIF_F_RXCSUM | NETIF_F_RXHASH | NETIF_F_GRO |
NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_CTAG_RX |
NETIF_F_HW_TC;
diff --git a/drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c b/drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c
index 3300b69a42b3..bcd38ea674ae 100644
--- a/drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c
@@ -1932,6 +1932,8 @@ static void cxgb4vf_get_wol(struct net_device *dev,
* TCP Segmentation Offload flags which we support.
*/
#define TSO_FLAGS (NETIF_F_TSO | NETIF_F_TSO6 | NETIF_F_TSO_ECN)
+#define VLAN_FEAT (NETIF_F_SG | NETIF_F_IP_CSUM | TSO_FLAGS | \
+ NETIF_F_GRO | NETIF_F_IPV6_CSUM | NETIF_F_HIGHDMA)
static const struct ethtool_ops cxgb4vf_ethtool_ops = {
.get_link_ksettings = cxgb4vf_get_link_ksettings,
@@ -3141,16 +3143,13 @@ static int cxgb4vf_pci_probe(struct pci_dev *pdev,
netif_carrier_off(netdev);
netdev->irq = pdev->irq;
- netdev->hw_features = NETIF_F_SG | TSO_FLAGS |
- NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
- NETIF_F_HW_VLAN_CTAG_RX | NETIF_F_RXCSUM;
- netdev->vlan_features = NETIF_F_SG | TSO_FLAGS |
- NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
- NETIF_F_HIGHDMA;
- netdev->features = netdev->hw_features |
- NETIF_F_HW_VLAN_CTAG_TX;
+ netdev->hw_features = NETIF_F_SG | TSO_FLAGS | NETIF_F_GRO |
+ NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM | NETIF_F_RXCSUM |
+ NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_CTAG_RX;
+ netdev->features = netdev->hw_features;
if (pci_using_dac)
netdev->features |= NETIF_F_HIGHDMA;
+ netdev->vlan_features = netdev->features & VLAN_FEAT;
netdev->priv_flags |= IFF_UNICAST_FLT;
netdev->min_mtu = 81;
--
2.9.5
^ permalink raw reply related
* [PATCH net-next 4/4] cxgb4: Enable outer UDP checksum offload for T6
From: Arjun Vynipadath @ 2019-02-28 9:39 UTC (permalink / raw)
To: netdev, davem; +Cc: nirranjan, indranil, dt, Arjun Vynipadath, Vishal Kulkarni
T6 adapters support outer UDP checksum offload for
encapsulated packets, hence enabling netdev feature flag
NETIF_F_GSO_UDP_TUNNEL_CSUM.
Signed-off-by: Arjun Vynipadath <arjun@chelsio.com>
Signed-off-by: Vishal Kulkarni <vishal@chelsio.com>
---
drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
index 8fd8e0849441..9b26c5b4d784 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
@@ -5719,9 +5719,11 @@ static int init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
NETIF_F_IPV6_CSUM |
NETIF_F_RXCSUM |
NETIF_F_GSO_UDP_TUNNEL |
+ NETIF_F_GSO_UDP_TUNNEL_CSUM |
NETIF_F_TSO | NETIF_F_TSO6;
netdev->hw_features |= NETIF_F_GSO_UDP_TUNNEL |
+ NETIF_F_GSO_UDP_TUNNEL_CSUM |
NETIF_F_HW_TLS_RECORD;
}
--
2.9.5
^ permalink raw reply related
* RE: [PATCH] xen-netback: fix occasional leak of grant ref mappings under memory pressure
From: Paul Durrant @ 2019-02-28 9:46 UTC (permalink / raw)
To: Igor Druzhinin, xen-devel@lists.xenproject.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: Wei Liu, davem@davemloft.net, Igor Druzhinin
In-Reply-To: <1551319382-32595-1-git-send-email-igor.druzhinin@citrix.com>
> -----Original Message-----
> From: Igor Druzhinin [mailto:igor.druzhinin@citrix.com]
> Sent: 28 February 2019 02:03
> To: xen-devel@lists.xenproject.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org
> Cc: Wei Liu <wei.liu2@citrix.com>; Paul Durrant <Paul.Durrant@citrix.com>; davem@davemloft.net; Igor
> Druzhinin <igor.druzhinin@citrix.com>
> Subject: [PATCH] xen-netback: fix occasional leak of grant ref mappings under memory pressure
>
> Zero-copy callback flag is not yet set on frag list skb at the moment
> xenvif_handle_frag_list() returns -ENOMEM. This eventually results in
> leaking grant ref mappings since xenvif_zerocopy_callback() is never
> called for these fragments. Those eventually build up and cause Xen
> to kill Dom0 as the slots get reused for new mappings.
>
> That behavior is observed under certain workloads where sudden spikes
> of page cache usage for writes coexist with active atomic skb allocations.
>
> Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com>
> ---
> drivers/net/xen-netback/netback.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
> index 80aae3a..2023317 100644
> --- a/drivers/net/xen-netback/netback.c
> +++ b/drivers/net/xen-netback/netback.c
> @@ -1146,9 +1146,12 @@ static int xenvif_tx_submit(struct xenvif_queue *queue)
>
> if (unlikely(skb_has_frag_list(skb))) {
> if (xenvif_handle_frag_list(queue, skb)) {
> + struct sk_buff *nskb =
> + skb_shinfo(skb)->frag_list;
> if (net_ratelimit())
> netdev_err(queue->vif->dev,
> "Not enough memory to consolidate frag_list!\n");
> + xenvif_skb_zerocopy_prepare(queue, nskb);
> xenvif_skb_zerocopy_prepare(queue, skb);
> kfree_skb(skb);
> continue;
Whilst this fix will do the job, I think it would be better to get rid of the kfree_skb() from inside xenvif_handle_frag_list() and always deal with it here rather than having it happen in two different places. Something like the following...
---8<---
diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index 80aae3a32c2a..093c7b860772 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -1027,13 +1027,13 @@ static void xenvif_tx_build_gops(struct xenvif_queue *queue,
/* Consolidate skb with a frag_list into a brand new one with local pages on
* frags. Returns 0 or -ENOMEM if can't allocate new pages.
*/
-static int xenvif_handle_frag_list(struct xenvif_queue *queue, struct sk_buff *skb)
+static int xenvif_handle_frag_list(struct xenvif_queue *queue, struct sk_buff *diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index 80aae3a32c2a..093c7b860772 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -1027,13 +1027,13 @@ static void xenvif_tx_build_gops(struct xenvif_queue *qu
eue,
/* Consolidate skb with a frag_list into a brand new one with local pages on
* frags. Returns 0 or -ENOMEM if can't allocate new pages.
*/
-static int xenvif_handle_frag_list(struct xenvif_queue *queue, struct sk_buff *
skb)
+static int xenvif_handle_frag_list(struct xenvif_queue *queue, struct sk_buff *
skb,
+ struct sk_buff *nskb)
{
unsigned int offset = skb_headlen(skb);
skb_frag_t frags[MAX_SKB_FRAGS];
int i, f;
struct ubuf_info *uarg;
- struct sk_buff *nskb = skb_shinfo(skb)->frag_list;
queue->stats.tx_zerocopy_sent += 2;
queue->stats.tx_frag_overflow++;
@@ -1072,11 +1072,6 @@ static int xenvif_handle_frag_list(struct xenvif_queue *q
ueue, struct sk_buff *s
skb_frag_size_set(&frags[i], len);
}
- /* Copied all the bits from the frag list -- free it. */
- skb_frag_list_init(skb);
- xenvif_skb_zerocopy_prepare(queue, nskb);
- kfree_skb(nskb);
-
/* Release all the original (foreign) frags. */
for (f = 0; f < skb_shinfo(skb)->nr_frags; f++)
skb_frag_unref(skb, f);
@@ -1145,7 +1140,11 @@ static int xenvif_tx_submit(struct xenvif_queue *queue)
xenvif_fill_frags(queue, skb);
if (unlikely(skb_has_frag_list(skb))) {
- if (xenvif_handle_frag_list(queue, skb)) {
+ struct sk_buff *nskb = skb_shinfo(skb)->frag_list;
+
+ xenvif_skb_zerocopy_prepare(queue, nskb);
+
+ if (xenvif_handle_frag_list(queue, skb, nskb)) {
if (net_ratelimit())
netdev_err(queue->vif->dev,
"Not enough memory to consolidate frag_list!\n");
@@ -1153,6 +1152,10 @@ static int xenvif_tx_submit(struct xenvif_queue *queue)
kfree_skb(skb);
continue;
}
+
+ /* Copied all the bits from the frag list. */
+ skb_frag_list_init(skb);
+ kfree(nskb);
}
skb->dev = queue->vif->dev;
---8<---
What do you think?
Paul
> --
> 2.7.4
^ permalink raw reply related
* Re: [Xen-devel] [PATCH] xen-netback: fix occasional leak of grant ref mappings under memory pressure
From: Andrew Cooper @ 2019-02-28 9:50 UTC (permalink / raw)
To: Igor Druzhinin, xen-devel, netdev, linux-kernel
Cc: paul.durrant, wei.liu2, davem
In-Reply-To: <1551319382-32595-1-git-send-email-igor.druzhinin@citrix.com>
On 28/02/2019 02:03, Igor Druzhinin wrote:
> Zero-copy callback flag is not yet set on frag list skb at the moment
> xenvif_handle_frag_list() returns -ENOMEM. This eventually results in
> leaking grant ref mappings since xenvif_zerocopy_callback() is never
> called for these fragments. Those eventually build up and cause Xen
> to kill Dom0 as the slots get reused for new mappings.
Its worth pointing out what (debug) Xen notices is dom0 performing
implicit grant unmap.
~Andrew
^ 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